[ajitpratap0]

Summary

Follow-ups from the pre-merge review of #866–#868. A missing ; no longer lets one statement silently swallow the next, and utility statements are accepted only in the dialects that have them.

• Statements that contain a statement now end at their grammar, so a following statement without ; is an error instead of being folded in. Covered: PREPARE … AS, DECLARE … CURSOR FOR, CREATE FUNCTION/PROCEDURE/TRIGGER/EVENT/RULE/TASK with a single-statement or quoted body, Snowflake PIPE/ALERT/STREAM, ClickHouse CREATE FUNCTION … AS (x) -> …, and LOAD DATA. Example: PostgreSQL PREPARE s AS SELECT 1\nDROP TABLE t used to be one statement, hiding the DROP from the scanner and linter.
• Closed grammars for GRANT/REVOKE (per-dialect options, grantees, [bracketed] names, MySQL REQUIRE/resource options/CURRENT_USER(), MariaDB IDENTIFIED VIA, Snowflake APPLICATION/IDENTIFIER()), PostgreSQL COPY (old and new option syntax, E''/$$ file names, DELIMITERS) and CALL. GRANT … TO u DROP TABLE t is now an error.
• Per-dialect gating of utility statements: DO, VACUUM, LOAD DATA (including BigQuery FROM FILES), PRAGMA, ATTACH, OPTIMIZE, FLUSH, KILL, SHOW, and GRANT/SET in SQLite. The generic dialect keeps accepting them all.
• PostgreSQL ALTER DEFAULT PRIVILEGES … GRANT/REVOKE parses and round-trips.
• :=: MySQL accepts it only in assignments (SET @v := …, SELECT @v := …). PostgreSQL f(a := 1) is parsed as a named argument and rendered f(a => 1).
• MariaDB DELETE `history` FROM history keeps its quotes, so it no longer re-parses as DELETE HISTORY.
• T-SQL IF … ELSE IF … chains parse iteratively in linear time. 200 branches used to exceed the depth limit and swallow the next statement.
• Shared parseScriptItem helper for both top-level parse loops, with tests for a GO line inside BEGIN…END, a CTE, a subquery, the context path and recovery.

Test Plan

• gofmt, go vet, golangci-lint run (0 issues), task test:race, task test:cbinding; TestRealWorldCorpus green with known_failures.txt empty
• New table-driven tests in utility_statement_end_test.go, plus GO-line, ELSE IF and named-argument tests
• Independent review: about 8,000 SQL strings from testdata and test files, compared per dialect against main. All remaining accept→reject differences are intentional (dialect gating, MySQL SELECT a := 1, Oracle-style routines outside generic/oracle). No quadratic cases (n vs 16n over 39 generators) and no panics (30k truncations).
• Parser benchmarks: allocs/op identical, ns/op within noise

Breaking Changes

Behaviour changes, listed in the CHANGELOG:

• Input that used to fold two statements into one is now an error.
• Utility statements in dialects that lack them are now rejected.
• MySQL := outside assignments is now rejected.
• PostgreSQL named arguments render as =>.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

[vercel]

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

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 27, 2026 7:55pm UTC

[github-actions]

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

---

Review of PR #872

I read the diff for the core parser changes: parser.go, tsql_control_flow.go, dml_delete.go, expressions_primary_ext.go and the PRAGMA gating. I did not read the two new large files in depth (utility_body.go, utility_grammar.go), and I did not run the test suite. The origin/main...HEAD diff failed here because the checkout has no merge base, so I diffed HEAD~2..HEAD instead.

What looks good

• parseScriptItem: it removes the duplicated loop body from parseTokens and parseContextTokens. Behaviour is preserved, including batchLineError and the optional ; or noteStatementEnd. A GO separator still skips the ; handling because it returns early. The context path no longer drifts from the main path.
• tsqlConditionalEnd: turning the ELSE IF recursion into a loop is correct. Each ELSE IF is the last statement of the IF before it, so the chain ends where its last branch ends. This removes the depth-limit problem for chains of about 200 branches and is linear. The ; before ELSE is still handled on each iteration.
• **MariaDB DELETE \history` FROM history:** historyTableQuotechecks that the token is quoted, is namedhistory, and is followed by FROM. The quote is kept on TableNameQuote, so the output no longer re-parses as DELETE HISTORY`. The scope is narrow.
• isUserVariable: it correctly excludes @@sys variables and ? placeholders when restricting :=.
• Tests: the PR adds table-driven tests and an 8,000-string differential review against main. The review recorded no quadratic cases and no panics, which is useful evidence for a parser change of this size.

Suggestions and risks (non-blocking)

1. PRAGMA gating in parser.go: p.alterDialect(dialectSQLite) is used for gating. Please confirm it returns true when the dialect is empty or generic, as the PR description says. A test that PRAGMA is rejected under PostgreSQL, MySQL and SQL Server, and accepted under SQLite and generic, would pin this down. The same applies to the other gated statements (DO, VACUUM, ATTACH, OPTIMIZE, FLUSH, KILL, SHOW).
2. historyTableQuote default quote: when currentIdentQuote() returns 0, the code falls back to `. That is right for MariaDB, but a DELETE "history" FROM history under ANSI_QUOTES would be rewritten with backticks. This is an edge case, but it is worth a comment or a test.
3. Breaking behaviour: rejecting SELECT a := 1 in MySQL, utility statements in dialects that lack them, and Oracle-style routines outside generic/oracle can break users of the library. The CHANGELOG entry covers it. Since the ! in the PR title already signals this, consider also noting it in the migration docs and in docs/SQL_COMPATIBILITY.md.
4. => rendering for PostgreSQL named arguments: f(a := 1) now round-trips to f(a => 1). Both are valid in PostgreSQL, so this is fine. A formatter golden test would make the change explicit, if one isn't already there.
5. Size: about 1,300 lines of new grammar code across utility_body.go and utility_grammar.go is a lot to review. The corpus ratchet (known_failures.txt empty) and the differential run make me more comfortable. Please make sure the fuzz or truncation tests (30k truncations) are committed rather than one-off, so regressions in statement-boundary handling are caught later.

Performance and security: the allocs/op figures are identical and ns/op is within noise. Ending statements at their grammar is a security improvement, because PREPARE … AS SELECT 1\nDROP TABLE t can no longer hide the DROP from the scanner and linter. I found no new concerns.

Verdict: the design is sound and the diff I read matches the PR description. I have no blocking findings, and the suggestions above are mostly extra tests for the dialect gating.

[github-actions]

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

---

Review of PR #872

I read the core parser changes and spot-checked two points from the earlier review against the code. I did not run the test suite, and I did not read utility_body.go or utility_grammar.go line by line.

Checked against the code

• alterDialect and the generic dialect: alterDialect (pkg/sql/parser/alter_dialect.go:77) returns alterMask()&m != 0. There is also an alterDialectStrict variant that returns false when no dialect is set (dialectAll). So the generic or empty dialect is accepted only if alterMask() returns dialectAll there, and each gate must use the non-strict variant. I did not confirm the mask value. A table test over {generic, empty, postgres, mysql, sqlserver, sqlite, …} × {PRAGMA, DO, VACUUM, ATTACH, OPTIMIZE, FLUSH, KILL, SHOW, LOAD DATA} would pin this down. Without one, picking the wrong variant at any call site is a silent regression.
• historyTableQuote (dml_delete.go:154): the earlier concern about ANSI_QUOTES is mostly moot. The function applies only when isMariaDB() is true. It returns the real quote from currentIdentQuote() and falls back to ` only when that is 0. The fallback comment should say when that case occurs, because it looks unreachable for a quoted identifier.

What looks good

• parseScriptItem removes the duplicated loop body, so the main and context paths no longer drift.
• tsqlConditionalEnd handles ELSE IF chains iteratively. Running time is linear, and chains of about 200 branches no longer hit the depth limit.
• Ending PREPARE … AS, DECLARE … CURSOR FOR and CREATE FUNCTION bodies at their grammar closes a real scanner and linter bypass, as in PREPARE s AS SELECT 1\nDROP TABLE t.
• Test evidence is strong: the corpus ratchet is empty, there is a differential run against main, and benchmark allocs/op are identical.

Suggestions (non-blocking)

1. Gating matrix test: add the dialect × statement test described above. It is the main coverage gap, since the PR gates many statements across 11 dialect names.
2. Truncation and scaling checks: the 30k-truncation (no panics) and n vs 16n (no quadratic cases) checks are described as one-off. Commit a smaller version, such as a truncation loop over the table-test inputs and a fuzz seed corpus, so boundary regressions are caught later.
3. Docs for breaking changes: SELECT a := 1 in MySQL, utility statements in dialects that lack them, and Oracle-style routines outside generic/oracle are now rejected. Add these to docs/SQL_COMPATIBILITY.md as well as the CHANGELOG.
4. Named-argument rendering: add a formatter golden test for f(a := 1) → f(a => 1). The roundtrip test added in pkg/gosqlx may already cover it.
5. Review size: about 1,260 lines of new grammar code. The CHANGELOG and tests help, but a short header comment in each new file describing the "closed grammar" approach would make future edits safer.

Performance and security: no new concerns. Parsing stays linear, and ending statements at their grammar is a security improvement.

Verdict: no blocking issues. The main follow-up is the dialect gating matrix test.
· branch fix/parser-statement-boundaries