MemberJunction / MemberJunction/MJ
PostgreSQL auto-quote: guard the ALL-CAPS-column / keyword-set invariant in CI
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 308
Description
Follow-up from the review of #3697, filed as agreed there. Not a defect today — a load-bearing invariant that is currently asserted in prose rather than in CI.
## The invariant
`AutoQuotePostgreSQLIdentifiers` evaluates its ALL-CAPS keyword tier **before** the dot rule. That ordering is deliberate and correct: several entries exist specifically for their dot-qualified form (`INFORMATION_SCHEMA.COLUMNS`, `.TABLES`, `.ROUTINES`), the catalog's real relation names are lower case, and quoting the right-hand half yields `INFORMATION_SCHEMA."COLUMNS"`, which does not resolve. CodeGen executes that exact SQL through `qsql()` on every PostgreSQL run, so an unconditional dot rule would turn a working CodeGen run into a hard failure.
The cost of that ordering is a standing invariant:
> **No ALL-CAPS column name in any shipped schema may appear in `PostgreSQLQuotingKeywords`.**
If one ever does, tier 1 swallows it *even when dot-qualified* — `t.ACTION` comes back bare, folds to `t.action`, and fails with `column "action" does not exist`. That is precisely the failure class #3697 exists to eliminate, reachable through the one door left open.
## Why it holds today, and why that is fragile
The invariant currently holds, and #3697's changeset states so. I verified it against `migrations-pg/v5/B202607091514__v5.46.x__Baseline.pg.sql` — the complete set of ALL-CAPS columns in the shipped schema is:
```
BCMID ID ISO2 ISO3 ISO3166_2 SQL URI URL
```
None is in the keyword set, and all eight quote correctly in both positions:
```
SELECT SQL FROM t => SELECT "SQL" FROM t
SELECT t.SQL FROM x t => SELECT t."SQL" FROM x t
```
The fragility is that the invariant can be broken from **either side**, by someone who has no reason to be looking at the other:
- a migration adds an ALL-CAPS column that happens to be a keyword (`ACTION`, `TEXT`, `LOG`, `RANK`, `VALUES` — all already in the set as ALL-CAPS entries), or
- someone adds a word to `PostgreSQLQuotingKeywords` to fix an unrelated keyword-quoting problem, which is a routine and otherwise-safe edit.
The second is the likely one. #3697 documents that adding a word to that set is free *because* the mixed-case column form still quotes — which is true for mixed-case columns and silently false for ALL-CAPS ones.
## Proposed guard
The same shape as the structural-tier guard `postgresqlAutoQuote.baseline.test.ts` already carries, in the same file, over the column set it already extracts:
```ts
it('no ALL-CAPS baseline column collides with the ALL-CAPS keyword tier', () => {
const colliding = distinctColumns
.filter((c) => c === c.toUpperCase())
.filter((c) => PostgreSQLQuotingKeywords.has(c));
expect(colliding, colliding.length === 0 ? '' :
`These ALL-CAPS baseline columns are in PostgreSQLQuotingKeywords: ${colliding.join(', ')}.\n\n` +
`Tier 1 runs before the dot rule, so such a column is emitted bare even as \`alias.COL\`, ` +
`folds to lowercase on PostgreSQL, and fails with 'column "..." does not exist'. Either ` +
`rename the column, or remove the word from PostgreSQLQuotingKeywords and confirm nothing ` +
`relies on its ALL-CAPS dot-qualified form (INFORMATION_SCHEMA.COLUMNS is the reason the ` +
`ordering exists).`
).toEqual([]);
});
```
Worth pairing with a positive assertion that the eight known ALL-CAPS columns still quote both bare and dot-qualified, so the guard fails on a rule regression as well as on a set/schema collision.
## Scope, stated honestly
This inherits the same limitation as the guards beside it, and the limitation should be repeated rather than assumed: it reads the **newest baseline only**. Columns added by later `migrations-pg` files and non-`__mj` schemas are not covered, so a green run means "no collision as of the last baseline", not "no collision". That is adequate here for the same reason it is adequate for the structural tier — the population is tiny and slow-moving — but it is not the same thing as proof.
Refs #3697.
Contributor guide
Research direction
Start in postgresqlAutoQuote.baseline.test.ts and inspect how it extracts columns from migrations-pg/v5/B202607091514__v5.46.x__Baseline.pg.sql. Add the invariant guard and the proposed positive coverage for the eight known ALL-CAPS columns, then run the relevant PostgreSQL auto-quote test or CI check; done means both assertions pass while retaining the newest-baseline limitation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, typescript
- Domain
- ci-cd, databases, testing
- Issue type
- Feature
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100