[ajitpratap0]

Summary

#601 ast/formatter: NOT EXISTS (subquery) renders as NOT (EXISTS (...) NOT ), which does not re-parse

This fixes #601. BinaryExpression.writeSQL (pkg/sql/ast/sql.go) now renders the parser's NOT EXISTS shape (Operator "NOT", Right nil, Left ExistsExpression) as a prefix NOT EXISTS (...). Before, it produced the invalid NOT (EXISTS (...) NOT ), which failed to re-parse with E2002. AST.SQL(), formatter.FormatAST (compact and readable) and formatter.FormatString all get the fix through the same render path. Neither the parser nor the AST shape changed. Restructuring NOT EXISTS as UnaryExpression or as an ExistsExpression.Not flag was deliberately left out, because it would be an observable AST change for the linter, security scanner and transform consumers. New regression tests check round-trips on 10 query shapes across the default and 8 dialect settings, plus the AST shape. They fail on 61ca6c8 and pass after the fix. The full race suite, go vet and golangci-lint are clean.

#611 ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names)

Fixes #611. ast.Identifier gains an additive Quoted bool, set by the parser for ANSI double-quoted names and cleared on pool release. Identifier.SQL() re-quotes those names. CTE names and column lists, CREATE [MATERIALIZED] VIEW names and column lists, and FROM/JOIN table names now go through new ast.QuoteName / ast.QuoteQualifiedName helpers, in both pkg/sql/ast and pkg/formatter. The helpers keep SQL Server #temp, Snowflake @stage/path and Unicode combining-mark names bare. The 5 inputs from the issue now render exactly as written in all 9 dialects through SQL() and FormatAST compact. Each re-parses to the same SQL(). The new regression tests fail on the parent commit. A differential run over about 3,600 corpus queries × 9 dialects found 0 round-trip regressions and 435 newly round-tripping outputs. Known costs: parser B/op rises 4–6% (Identifier size class 48 to 64 B, same allocation count) and SQL() is about 8% slower. Fingerprint normalization now keeps double quotes, so "id" and id produce different fingerprints. Deferred follow-ups, each with a sound scope reason:

• case preservation in string-typed name positions (FROM "Users", CTE/view names, aliases)
• quoting of qualifier parts ("T"."c")
• Quoted flag in INSERT/UPDATE/USING/PIVOT/DDL column lists (files owned by the parser: INSERT gaps: RETURNING after INSERT ... SELECT, DEFAULT VALUES, MySQL INSERT ... SET, INSERT INTO t (SELECT ...) #610 and parser/ast: JOIN ... USING (cols) is stored as a plain condition and renders as ON cols #603 chains)
• dialect-aware backtick/bracket output
• the CLI format path (comes in with cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599)

#604 ast/formatter: SQL Server table hints WITH (NOLOCK) are never rendered; SQL() also drops PIVOT

ast SQL() now renders SQL Server table hints as WITH (h1, h2) and renders PIVOT/UNPIVOT with the alias as AS alias, in parser order: alias, FINAL, FOR SYSTEM_TIME, hints, PIVOT/UNPIVOT, AS alias. Before this, SELECT * FROM s PIVOT (...) AS p rendered as SELECT * FROM s p and WITH (NOLOCK) was dropped. pkg/formatter (and so gosqlx.FormatAST) now emits hints between a plain alias and PIVOT/UNPIVOT, following the keyword-case setting. Output for table references without hints or pivots is byte-identical; the only expectation change is the select/everything golden gaining WITH (NOLOCK). Verification: four new regression tests fail on the parent commit and pass at HEAD. An independent check re-parsed the output of SQL(), compact, zero-opts and readable formatter and gosqlx.FormatAST for sqlserver, oracle and snowflake, and the full race gate, vet and golangci-lint are clean. Deferred, with sound reasons: the CLI renderer is fixed by #599, so link it or close #604 after #599 merges. Follow-ups: Oracle/Snowflake string literals lose their quotes in PIVOT IN lists (parser); a derived-table alias is overwritten by the pivot alias (needs an AST field); hints on UPDATE/DELETE/MERGE and parenthesized hints like INDEX(ix) don't parse; dialect-aware PIVOT alias and identifier quoting; the formatter's missing FINAL/FOR SYSTEM_TIME/TableFunc rendering (#607 and others).

Commits (5), one implementation commit per issue plus test-only follow-ups:

• 9b56c7d fix(ast,formatter): render SQL Server table hints and PIVOT/UNPIVOT in SQL() and FormatAST (ast/formatter: SQL Server table hints WITH (NOLOCK) are never rendered; SQL() also drops PIVOT #604)
• cc23f9b fix(ast): address review for ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611
• 490c204 fix(ast): address review for ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611
• 3b0619b fix(ast,formatter,parser): preserve quoted identifiers that are reserved words or case-sensitive on render (ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611)
• 4d1c4fb fix(ast): render NOT EXISTS as a prefix NOT so SQL() and formatter output re-parse (ast/formatter: NOT EXISTS (subquery) renders as NOT (EXISTS (...) NOT ), which does not re-parse #601)

19 files changed, 1585 insertions(+), 42 deletions(-)

Test Plan

• Each issue's reproduction cases were run against main before the fix (Haiku scout), and a failing regression test was written before the code change.
• Independent review (Opus, or Sonnet on small chains) with a bounded fix loop.
• Independent Opus validator ran the repository race gate (CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding)), go vet, golangci-lint, re-ran the reproduction cases on HEAD, and confirmed the new tests fail on the parent commit.
• Perf items: benchstat -count=6 before/after on the same machine.

#601 evidence (validator)

Not a perf item, so no benchmarks were run. Repro evidence: a scratch module that uses a replace directive pointing at the worktree checked 3 scout cases plus a control query at HEAD.
- **Scout cases:** `SELECT * FROM t WHERE NOT EXISTS (SELECT 1 FROM u)`, the `... AND a = 1` variant and the correlated `a = 1 AND NOT EXISTS (... WHERE u.x = t.x)` variant. For each one, tree.SQL() and FormatAST(CompactStyle) are byte-identical to the input. FormatString gives `SELECT *\nFROM t\nWHERE NOT EXISTS (SELECT 1 FROM u);` (and the matching form for the other two). All three outputs re-parse, and the re-parsed SQL() equals the first SQL().
- **Control query:** IS NOT NULL / NOT LIKE / NOT ILIKE / NOT active also renders unchanged and round-trips.
- **Pre-fix failure:** I ran git archive on parent 61ca6c8, added the 2 new test files and ran them. TestBinaryExpression_NotExistsSQL_Issue601 fails with SQL() = "NOT (EXISTS (SELECT 1 FROM u) NOT )". TestNotExists_RoundTrip_Issue601 and TestNotExists_FormatString_Issue601 also fail, 92 FAIL lines in total. At HEAD all of them pass.
- **CLI scout case:** it was already correct before the fix and is out of scope (the CLI formatter was fixed in #591/#587).

#601 race gate: I ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) at HEAD 4d1c4fb and it exited with code 0. 44 packages reported ok and none failed or flaked. The log is at /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/601/race.txt.
#611 evidence (validator)

This is a bug fix, not a perf change, but I benchmarked it because the parser and Identifier are hot paths. Setup: interleaved runs, 9 samples each, parent 4d1c4fb vs HEAD cc23f9b, Apple M4.
- ParserSimpleSelect: time ~ (p=0.945); B/op 816 -> 848 (+3.9%); allocs 7 -> 7.
- ParserComplexSelect: time +0.88% (p=0.004); B/op 1.922Ki -> 2.031Ki (+5.7%); allocs 20 -> 20.
- ast SQL/7line: time 426n -> 464n (+8.9%); B/op and allocs unchanged (240 B, 1 alloc).
- ast SQL/15line: time 831n -> 889n (+7.0%); B/op and allocs unchanged (576 B, 1 alloc).

Causes:
- The B/op rise comes from the new Identifier.Quoted bool, which the plan approved. It pushes the struct from 48 B (Name + Table + Location{int,int}) into the 64 B size class. Allocation counts do not change.
- The SQL() slowdown comes from table names now going through writeQualifiedName / nameNeedsQuoting, a per-rune scan plus a reserved-word map lookup that does not allocate.

Round-trip and repro evidence:
- All 5 scout cases render exactly as the input through stmt.SQL() and FormatAST compact in all 9 dialects. Each re-parses, and SQL() of the re-parse is identical. Before the fix the outputs were 'WITH c (select) ...', 'WITH c (a b) ...', 'CREATE VIEW v (select) ...', 'SELECT select FROM t' and 'SELECT "a b", Mixed FROM my table'.
- The new regression tests fail on the parent. I ran git archive of 4d1c4fb, stubbed only the Quoted field so the tests compile, and dropped the unit tests that need the new helpers. TestQuotedIdentifierRender_Issue611, TestIdentifierQuotedFlag_Issue611 and TestQuotedIdentifier_FormatAST_Issue611 then FAIL. The no-change guard tests (DialectGuards, SnowflakeStagePath, CombiningMarks) PASS there, as they should.
- Corpus differential: 3,638 SQL strings taken from every pkg/**/_test.go string literal and .sql file, plus 65 edge cases, each parsed in 9 dialects. 921 outputs changed. Every changed input I traced wrote the affected names in double quotes, or in MySQL backticks / SQL Server bracke

#611 race gate: PASS. Command: CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) at HEAD cc23f9b. Exit 0; 44 packages ok, 0 FAIL, no flakes, so nothing was re-run. Log: /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/611/race.txt. TestPerformanceRegression (pkg/sql/parser, run without -race) also passes.
#604 evidence (validator)

Not a perf item. Bug-fix evidence from an independent scratch module (replace directive pointing at the worktree, /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/604/repro/main.go). Each case was rendered five ways (SQL(), formatter compact, formatter zero FormatOptions, formatter readable, gosqlx.FormatAST), then re-parsed and compared with the original SQL(). All renders round-trip (RT-OK) for: sqlserver `SELECT * FROM t WITH (NOLOCK)` -> `SELECT * FROM t WITH (NOLOCK)` (it was `SELECT * FROM t`); sqlserver `SELECT * FROM s PIVOT (SUM(amt) FOR q IN ([Q1], [Q2])) AS p` -> identical (it was `SELECT * FROM s p`); `t x WITH (NOLOCK)`; a LEFT JOIN right side with `WITH (READPAST, ROWLOCK)`; `t WITH (NOLOCK) PIVOT (...) AS p`; sqlserver UNPIVOT; oracle PIVOT/UNPIVOT; snowflake PIVOT/UNPIVOT; a derived table `(SELECT 1 AS a) d WITH (NOLOCK)`. Inputs without hints or pivots are unchanged (pg JOIN alias, mysql derived table). Parent check: git archive of cc23f9b (the parent of 9b56c7d) with the two new test files copied in. All 4 new tests FAIL there (TestTableHintsPivot_SQLRoundTrip_Issue604, TestTableRef_HintsPivot_HandBuilt_Issue604, TestTableHints_FormatAST_Issue604, TestTableHints_FormatAST_KeywordLower_Issue604), and all 4 PASS at HEAD.

#604 race gate: PASS. Ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) at HEAD 9b56c7d: EXIT=0, 44 packages ok, no FAIL, panic or DATA RACE. Log: /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/604/race.txt

Breaking Changes

• ast/formatter: NOT EXISTS (subquery) renders as NOT (EXISTS (...) NOT ), which does not re-parse #601: none. No exported signatures or AST fields change. The only observable change: BinaryExpression.SQL() (and so ast.AST.SQL(), formatter.FormatAST/FormatStatement/FormatExpression and FormatString) now renders the parser's NOT EXISTS shape (Operator "NOT", Right nil) as NOT EXISTS (...) instead of the invalid NOT (EXISTS (...) NOT ). This is a bug fix; the previous output could not be re-parsed.
• ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611: Additive only, nothing removed or re-signed:
• New field ast.Identifier.Quoted bool, set by the parser for ANSI double-quoted identifiers and cleared on pool release.
• New exported helpers ast.QuoteName(string) string and ast.QuoteQualifiedName(string) string, needed because pkg/formatter cannot call unexported ast helpers.

Observable behaviour changes in SQL(), FormatAST, FormatStatement, Normalize, Fingerprint and Transpile output:

• Identifiers written with double quotes keep them.
• CTE names and column lists, CREATE [MATERIALIZED] VIEW names and column lists, and FROM/JOIN table names are double-quoted when they contain non-identifier characters or are a reserved word the parser cannot accept bare there.
• Unquoted input renders as before.
• ast/formatter: SQL Server table hints WITH (NOLOCK) are never rendered; SQL() also drops PIVOT #604: No exported API changes. The observable change is additive: SQL() now emits WITH (hints) and the PIVOT/UNPIVOT clauses (with the pivot alias as AS alias), and the formatter now emits WITH (hints). Output for table references without hints or pivots is byte-identical. No new AST fields, so no pool-reset changes.

Render-time injection hardening (added during the merge train)

The adversarial and security review found that quoted names and some literal values were written into rendered SQL verbatim. So SELECT 1 AS "a; DROP TABLE x; --" FROM t came back from gosqlx.Format / SQL() as two statements. Main has the same exposure. This PR now closes the whole class:

• Names: SELECT, table, derived-table, JOIN, PIVOT, UPDATE/DELETE and MERGE aliases, window names, DML targets and column lists, and DDL names (CREATE TABLE/INDEX, DROP, TRUNCATE, ALTER, REFRESH, SHOW, DESCRIBE) are quoted when they are not plain, or when they are reserved. Constraint names, function names, data types, INTERVAL values, EXTRACT fields and PRAGMA are covered too.
• Tokenizer: a token opened with an ASCII ' or " now closes only on that same ASCII quote. Smart quotes such as ” and ’ inside it are ordinary content. They used to end the token on re-parse, which allowed injection through SELECT 1 AS x”; DROP…“``.
• Keyword fields (join type, DROP object type, set operator, interval qualifier, null treatment, constraint/partition/ALTER/MERGE types, table options, view WITH option, SHOW kind, frame and lock types, FK actions, FETCH, SAMPLE) are checked against exact allow-lists. Every value the parser produces is on the lists, and a guard test enforces that. A hand-built value outside its list is dropped, or written so that the output fails to parse. It can never add a statement.
• Formatter: the -- UNSUPPORTED: comment now comments out every line, so a newline in a literal can't start a live statement.

Validation (independent Opus validators, 4 rounds):

• Template fuzzer: 549k hostile-name cases × 9 dialects × 4 renderers. 0 injected statements; main has 5,538.
• Mutation fuzzer over hand-built ASTs, including word-only payloads: 0 injectable paths; 327 before the last round.
• Tokenizer differential against main: 19.5k inputs × 9 dialects. The only differences are smart quotes inside ASCII-quoted tokens.
• Render differential against the pre-allow-list head: 191k records, byte-identical. Tokenizer and parser benchmarks are within ±1% geomean, with allocations unchanged.

Additional behaviour changes:

• T-SQL table hints must be bare words: WITH (INDEX = ix) is now a parse error. It used to be accepted and the hint silently dropped.
• CONSTRAINT "x y" (double-quoted constraint names) now parses in SQL Server. A name quoted only for its case, such as "MixedCase", renders bare, consistent with other identifiers.
• CAST(x AS "type") now parses. Quoted types that aren't plain keep their quotes.
• Smart quotes inside ASCII-quoted strings and identifiers are kept as written, instead of being turned into ' / ".
• A non-ASCII lıke is stored as LIKE. Window rendering no longer repeats the window name or wraps OVER w in parentheses. The formatter keeps WITH in WITH CHECK OPTION.

Follow-ups found during review, already on main and not changed here: ALTER TABLE ... ADD CONSTRAINT renders empty. SQL Server CROSS APPLY renders as CROSS APPLY JOIN. MySQL MATCH ... AGAINST duplicates AGAINST. In the default dialect, "#x" renders bare and fails to re-parse.

Notes

• CHANGELOG.md is intentionally not touched; the consolidated CHANGELOG PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 gets the entries for this batch before it merges.
• Part of the batch-3 merge train (follow-ups cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599-schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles) #620); later PRs in the train are rebased onto this one after it merges.
• Base: main.

Fixes #601
Fixes #611
Fixes #604
Follow-up to #566

🤖 Generated with Claude Code

https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 14, 2026 11:31am UTC

[github-actions]

Claude finished @ajitpratap0's task in 37s —— View job

---

PR review: #632 (NOT EXISTS, quoted identifiers, table hints/PIVOT render)

Verdict: looks good. I have a few non-blocking suggestions. I read the core changes and spot-checked the pooling and render paths. I did not run the test suite, and I didn't review every test file in detail. The PR description reports a clean race gate, vet and lint.

I couldn't diff against origin/main. The checkout has no merge base with it, so git diff origin/main...HEAD failed. I reviewed the files directly at HEAD instead.

What I checked

• Pool reset (ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611): Identifier.Quoted is cleared on release. PutExpression does *e = Identifier{}, so the new field needs no extra reset code. TestIdentifierPool_ResetsQuoted also covers it.
• Quoting helpers (pkg/sql/ast/quote_names.go):
  • isReservedName uses a stack buffer and doesn't allocate.
  • QuoteName and QuoteQualifiedName return the input unchanged when nothing needs quoting.
  • Embedded " is doubled correctly.
  • The reservedNameWords list is guarded by TestReservedNameWordsRejectedBare, so it will fail if the parser starts accepting a word bare. That was the main risk of a hard-coded list.
• Table ref rendering (pkg/sql/ast/sql.go:1969-2003, sql_table_hints_pivot.go): the order is alias, FINAL, FOR SYSTEM_TIME, hints, PIVOT/UNPIVOT, then AS alias. It matches the parser order described in the PR. Tables without hints or pivots render as before.

Suggestions (non-blocking)

1. maxReservedNameLen = 16 is a silent coupling. If someone adds a reserved word longer than 16 characters (for example CONCURRENTLY is 12 and AUTOINCREMENT is 13, so the current margin is small), it will never match. A test that asserts len(w) <= maxReservedNameLen for every key in reservedNameWords would make that fail loudly.
2. Dialect-specific over-quoting. The "reserved in every dialect" rule means a word is quoted even in dialects that would accept it bare. That is safe for round-tripping, because ANSI "..." parses everywhere according to your tests. It will look surprising in MySQL and SQL Server output, so a doc note would help. You already list dialect-aware output as a follow-up.
3. Fingerprint behavior change. "id" and id now fingerprint differently, which you call out in the PR. Consider a CHANGELOG line for PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 that names it explicitly, since downstream consumers who dedupe by fingerprint may see their bucket counts change.
4. Performance.
   • The +4–6% B/op comes from Identifier growing from 48 to 64 B. If the struct has any padding to reclaim, reordering fields might avoid the size-class bump. I haven't checked the layout, so this is speculative.
   • The ~8% SQL() slowdown is acceptable for a correctness fix.
5. PIVOT/UNPIVOT values are written verbatim (writePivotClause, writeUnpivotClause). That is fine because the parser preserves the delimiters through renderQuotedIdent. It does mean a hand-built AST with unquoted special characters won't be escaped. TestTableRef_HintsPivot_HandBuilt_Issue604 covers the hand-built case, but hint strings and PivotColumn are not validated. That's acceptable for an internal render path, but worth a doc comment.
6. Hints are written verbatim (writeTableHints). This is fine for parser output. A hand-built AST with an attacker-controlled hint string would be injected as-is. This isn't a regression, since the renderer already trusts string fields elsewhere.

Test coverage

Coverage looks strong:

• Round trips across the default dialect plus 8 dialects.
• Pre-fix failure confirmation for the new tests.
• Guards for dialects and the Snowflake stage and combining-mark cases.
• The golden test change is minimal (one line).

Security

No new injection surface. Quoting only adds escaping, and " is doubled correctly.

The PR is fine to merge from my side once you've considered point 1.

--- · Branch: fix/ast-render-not-exists-quotes-hints

[ajitpratap0]

Rebased onto main to clear conflicts; the only conflict (pkg/sql/ast/sql_golden_test.go select/everything) combines main's window-clause fix with this PR's WITH (NOLOCK). task check passes.

Note for whoever owns GoSQLX-worktrees/fix-ast-render-not-exists-quotes-hints: the untracked pkg/sql/ast/render_injection_632_test.go was NOT included because it fails (10 cases; the PIVOT/UNPIVOT quoting fix isn't written yet). Rebase onto the new head 9038ed0 before pushing.

[github-actions]

Claude finished @ajitpratap0's task in 17s —— View job

---

PR review: #632 (NOT EXISTS, quoted identifiers, table hints/PIVOT render)

Verdict: looks good to merge. I have only minor, non-blocking notes. I read the changed code. I did not run the test suite. The PR description reports a clean race gate, vet and lint.

Checked

• ast/formatter: NOT EXISTS (subquery) renders as NOT (EXISTS (...) NOT ), which does not re-parse #601: BinaryExpression.writeSQL renders the NOT + nil-Right + ExistsExpression shape as a prefix NOT EXISTS (...). There is no AST or parser change, and the tests cover round-trips.
• ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611, pooling: Identifier.Quoted is cleared on release because the pool does *e = Identifier{}, and a test covers it.
• ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611, quoting helpers: QuoteName and QuoteQualifiedName return the input unchanged when nothing needs quoting, and embedded " is doubled. TestReservedNameWordsRejectedBare guards the hard-coded reserved list against drifting from the parser.
• ast/formatter: SQL Server table hints WITH (NOLOCK) are never rendered; SQL() also drops PIVOT #604: the render order (alias, FINAL, FOR SYSTEM_TIME, hints, PIVOT/UNPIVOT, AS alias) matches the parser order. Tables without hints or pivots render as before.

Notes

1. maxReservedNameLen = 16: this is already guarded. quote_names_test.go:104 asserts every reserved word fits in it, so the earlier concern is covered.
2. Over-quoting: a word reserved in any dialect is quoted in every dialect, including MySQL and SQL Server, where it would parse bare. Round-trips stay safe. A short doc note would help, and dialect-aware output is already a listed follow-up.
3. Fingerprints: "id" and id now fingerprint differently. Name this explicitly in the CHANGELOG entry for docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598, because consumers who dedupe by fingerprint will see their bucket counts shift.
4. Verbatim strings: table hints and PIVOT/UNPIVOT values are written verbatim. That is fine for parser output, but a hand-built AST with untrusted strings would be injected as-is. A doc comment saying the renderer trusts those fields would be enough.
5. Performance:
   • The +4–6% B/op comes from Identifier going from 48 to 64 B. Reordering fields is unlikely to help, because adding a bool to string, string and Location{int,int} pushes the struct past 48 B regardless of order.
   • The ~8% SQL() slowdown is acceptable for a correctness fix.

Security

No new injection surface. Quoting only adds escaping.
· Branch: fix/ast-render-not-exists-quotes-hints

[github-actions]

Claude finished @ajitpratap0's task in 20s —— View job

---

PR review: #632 (NOT EXISTS, quoted identifiers, table hints/PIVOT render)

Verdict: I could only do a limited review, so treat this as a partial read. The sandbox denied go test and go build, and I couldn't diff against origin/main. I ran no tests and didn't read the new code in detail. The two earlier bot reviews on this PR covered the core render changes. I have no new findings to add.

What I did not cover

• The PR now has 14 commits and 45 files. The description still lists 5 commits and 19 files.
• The tokenizer, parser and quote_names.go (1228 lines) changes aren't in the description. They include:
  • pkg/sql/tokenizer/tokenizer.go, string_literal.go, unicode.go, and a changed tokenizer golden file;
  • pkg/sql/parser/expressions_*.go, pivot.go, select_subquery.go.
• I didn't check those files.

Suggestions

1. Update the PR description. Add the tokenizer and parser changes. The tokenizer golden file (tokenizer_literal_golden.json) changed, and that is a behaviour change reviewers should see called out.
2. Add a CHANGELOG note for PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 on fingerprints. Fingerprints now keep double quotes, so "id" and id fingerprint differently. Anyone who dedupes by fingerprint will see their buckets shift.
3. Consider a doc comment on the verbatim string fields. Table hints and PIVOT values are written as-is. That is fine for parser output. A hand-built AST with untrusted strings would be injected into the output.
4. Over-quoting is intentional but worth documenting. Reserved words are quoted in every dialect, including MySQL and SQL Server, where they would parse bare. Round-trips stay safe, and dialect-aware output is already a listed follow-up.
5. The Identifier size class is unavoidable. The 48 to 64 B growth comes from adding any field to the struct, so I wouldn't spend time reordering fields.

Before merge

• Please confirm task check is green on the rebased head, since I couldn't run it.
• The untracked render_injection_632_test.go was left out because the PIVOT/UNPIVOT quoting fix isn't written yet. Both pkg/formatter/render_injection_632_test.go and pkg/sql/ast/render_injection_632_test.go appear in the changed-files list. Please confirm they pass or are skipped, since the earlier comment says 10 cases fail.
  · Branch: fix/ast-render-not-exists-quotes-hints

[ajitpratap0]

Merge train (batch 3): #632

• Rebase: onto main 95978a4, which comes after fix(parser): linear qualified-name joining and correct E2002 error context (#613, #616) #627. Conflicts in sql.go, render.go and sql_golden_test.go against fix(parser,ast,formatter): named window round-trip, GROUPS/EXCLUDE frames and INTERVAL bounds (#608, #609) #629 were resolved by keeping main's BaseWindow / INTERVAL Qualifier structure and applying this PR's quoting on top. An independent validator checked the resolution. The head is e6be50f.
• Local gate on the final head: go vet ./..., golangci-lint (0 issues) and go test -race on every package except cbinding pass, as does the pre-commit hook. CI is 28/28 green.
• Adversarial and security pass: the first round confirmed 3 must-fix findings: table-hint injection, PIVOT literal injection, and alias/reference quoting mismatch. While fixing them, the review found that every quoted-name position, plus several literal and keyword fields, rendered verbatim, so SELECT 1 AS "a; DROP TABLE x; --" came back as two statements. Main has the same exposure. The PR now closes that class; see the "Render-time injection hardening" section of the description. It took four Opus fix/validate rounds:
  • Round 1, aliases and DDL names: ASCII payloads were closed, but smart quotes still terminated ASCII-quoted tokens. Fixed at the root in the tokenizer.
  • Round 2: a ClickHouse CODEC(...) type regression, plus raw keyword fields in hand-built ASTs. Fixed.
  • Round 3: word-only payloads, such as a JoinClause.Type with no ;, could still start a statement, because the parser accepts unseparated statements. Replaced with exact per-field allow-lists and a guard test proving every parser-produced value is listed.
  • Round 4: pass.
    • Mutation fuzzer: 0 injectable paths across 36,100 dialect/field pairs.
    • Template fuzzer: 0 injected statements over 549k cases.
    • Render differential versus the previous head: 191,592 records, byte-identical.
    • Tokenizer differential versus main: differs only on smart quotes inside ASCII-quoted tokens.
    • Benchmarks are within ±1% geomean, with allocations unchanged.
• Since round 4: e6be50f renders a hand-built MINUS as EXCEPT, which is equivalent and which the parser already uses, instead of the unparseable fallback. It has a test.
• Bot review on this head:
  • The description is now updated.
  • Fingerprints: "id" and id still fingerprint identically (7d3a655067d7 for both). Only reserved-word names change: "select" now normalizes to "select" instead of the invalid bare select. This will be noted in the CHANGELOG (docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598).
  • Hint and PIVOT values are no longer verbatim: they go through QuoteTableHint, QuotePivotColumn and QuotePivotItem, and string literals are escaped.
  • The render_injection_632_test.go files are committed and pass.
  • Dialect-aware quoting is a listed follow-up.
• Behaviour changes:
  • T-SQL WITH (INDEX = ix) is now a parse error. It used to be silently dropped.
  • SQL Server accepts CONSTRAINT "x y".
  • CAST(x AS "type") parses.
  • Smart quotes inside ASCII-quoted tokens are preserved.
• Follow-ups (already on main):
  • ALTER TABLE ... ADD CONSTRAINT renders empty.
  • CROSS APPLY renders as CROSS APPLY JOIN.
  • MATCH ... AGAINST renders a duplicated AGAINST.
  • Default-dialect "#x" renders bare.