MemberJunction / MemberJunction/MJ

PostgreSQL auto-quote tokenizer: comment and E'...' handling can swallow real SQL

Open
#3,775 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

Split out of @rkihm-BC's review of #3697:

> Now that the comment / `E'...'` limitations live in exactly one place, they're worth a follow-up
> issue rather than only a docblock. The apostrophe-in-a-comment case is the sharp one: it starts a
> string scan that swallows real SQL.

## Background

#3697 consolidated two divergent PostgreSQL identifier tokenizers into one
(`packages/SQLDialect/src/postgresqlAutoQuote.ts`). The limitations below are **pre-existing and
unchanged by that PR** — both prior copies had them. They are filed now because consolidation means
there is finally a single place to fix them.

## The sharp one: an apostrophe inside a comment

The tokenizer's string-literal scan does not know it is inside a comment, so an ordinary English
apostrophe starts a string that runs until the next quote character — swallowing whatever SQL lies
between:

```sql
-- don't quote this column
SELECT Name FROM t
```

Everything from `don'` onward is treated as a string literal, so `Name` is never examined and ships
bare. It then folds to `name` and fails. The failure is invisible at authoring time and depends on
the prose in a comment, which is about as far from the actual defect as a symptom can get.

## The other: `E'...'` escape strings

`E'\\''` and friends use backslash escaping, which the scanner does not model, so the literal's end
is mis-detected in the same way.

## Suggested shape of a fix

Handle comments as their own token class (`--` to end-of-line, `/* */` with nesting, which
PostgreSQL supports), consumed *before* the string scan, and teach the string scanner about the
`E''` form's backslash escapes. Both are local to the one module now.

## Not urgent

No in-repo occurrence is currently known — this is about hand-authored and stored SQL in consumer
databases. Filed so the limitation is tracked somewhere other than a docblock.

Contributor guide

Open the contributing guide

Research direction

Start in packages/SQLDialect/src/postgresqlAutoQuote.ts and trace the tokenizer's comment and string-literal handling. Reproduce the apostrophe-in-comment and E'...' cases, then verify that comments are skipped correctly, escape strings end at the right quote, and real SQL identifiers are still examined.

Written by the indexing model from the issue text.

Assessment

Tech stack
postgresql, typescript
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.