fix(parser): INSERT ... SELECT RETURNING, DEFAULT VALUES, MySQL INSERT ... SET and parenthesized sources (#610) by ajitpratap0 · Pull Request #623 · ajitpratap0/GoSQLX

ajitpratap0 · 2026-10-14T05:30:08Z

Summary

#610 parser: INSERT gaps: RETURNING after INSERT ... SELECT, DEFAULT VALUES, MySQL INSERT ... SET, INSERT INTO t (SELECT ...)

Fixes #610.

INSERT ... SELECT now accepts a trailing RETURNING clause. While the parser is inside an INSERT source query or an UPDATE ... FROM list, an unquoted RETURNING is no longer consumed as an implicit table or select alias. The context flag is restored on every path, including errors, and is cleared by Reset().

The commit also adds:

INSERT INTO t DEFAULT VALUES (all dialects).
MySQL/MariaDB INSERT ... SET, stored as Columns plus one Values row and marked with the new additive bool fields InsertStatement.DefaultValues and SetSyntax, which are reset in the pool release path.
Parenthesized INSERT sources, INSERT INTO t (SELECT ...).
UPDATE ... FROM with a comma-separated table list. A JOIN there returns E4001 unsupported.

SQL() and formatter.FormatAST render DEFAULT VALUES and the SET form.

Every issue case failed with E2002 on 61ca6c8, and the new regression tests fail on that parent. On HEAD they pass, and SQL() output is a parse fixpoint in pg, generic, sqlserver, sqlite, mysql and mariadb. The full race suite (excluding cbinding), go vet and golangci-lint are clean.

Deferred:

Readable-style formatting of INSERT with a nested query emits an inner ';' (formatter: readable style emits ; inside CTE/IN/EXISTS subqueries and set operations, and ;; after INSERT ... SELECT / CREATE VIEW #602).
The CLI formatter drops RETURNING (cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599).
Parenthesized set operations and trailing clauses after a paren source (parser: parenthesized set operations not accepted as derived tables or statements; ((SELECT ...)) t gives a misleading join-group error #612).
Quote-dropping on keyword identifiers (ast/formatter: quoted identifiers that are keywords or case-sensitive lose their quotes on render (SELECT "select", CTE/view column lists, table names) #611).
Follow-up issues are recommended for: ON DUPLICATE KEY UPDATE / OUTPUT rendering, JOINs in UPDATE ... FROM, DELETE ... USING, and case-insensitive RETURNING in UPDATE/DELETE.

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

f59b1a8 fix(parser,ast,formatter): accept RETURNING after INSERT ... SELECT, DEFAULT VALUES, MySQL INSERT ... SET, parenthesized INSERT sources and UPDATE ... FROM (parser: INSERT gaps: RETURNING after INSERT ... SELECT, DEFAULT VALUES, MySQL INSERT ... SET, INSERT INTO t (SELECT ...) #610)

12 files changed, 992 insertions(+), 25 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.

#610 evidence (validator)

This is not a perf item and no benchmarks were required. The only hot-path addition is a bool check in front of an existing string comparison in the alias conditions.

Before the fix: the new regression tests were run on parent 61ca6c8 via git archive into scratchpad/b3/610/parent, with compile-only shims (the DefaultValues and SetSyntax fields, and Parser.dmlReturning). They FAIL there:
- TestInsertGrammarGaps_ReturningAfterSelect, _DefaultValues, _SetSyntax, _ParenthesizedSource
- TestUpdateFrom, TestInsertGrammarGapsErrors, TestDMLReturningFlagRestored
- TestInsertStatementSQL_Forms, TestInsertStatementPool_ResetsFormFlags
- TestRenderInsertForms

TestReturningAliasOutsideDML passes on the parent, as expected: it guards against a behaviour change.

After the fix: an independent probe module (scratchpad/b3/610/probe) uses gosqlx.ParseWithDialect, then SQL(), then re-parse:
- `INSERT INTO t SELECT * FROM s RETURNING id` (pg): OK, SQL fixpoint, ret=1, fromAlias="".
- `INSERT INTO t (a) SELECT b FROM s RETURNING *` (pg): OK, fixpoint, ret=1.
- `INSERT INTO t DEFAULT VALUES` (generic, pg, sqlserver, sqlite): OK, fixpoint, DV=true. Compact and readable FormatAST both re-parse.
- `INSERT INTO t SET a = 1, b = 2` (mysql, mariadb): OK, fixpoint, SET=true, cols=2, rows=1. Both formatter styles re-parse.
- `INSERT INTO t (SELECT * FROM s)` (generic, pg, mysql, sqlserver): OK, renders as `INSERT INTO t SELECT * FROM s`, fixpoint, Q=true.
- `UPDATE t SET a = s.b FROM s WHERE t.id = s.id` (pg): OK, fixpoint, from=1. Both formatter styles re-parse.
- Control cases (VALUES ... RETURNING, AS x, WHERE ... RETURNING): unchanged and fixpoint.
- `SELECT * FROM s returning` (pg, generic): Alias="returning" is still kept.
- `INSERT INTO t1 (a) DEFAULT` and pg `INSERT INTO t SET a = 1`: still error with "VALUES, SELECT, or WITH".
- `UPDATE ... FROM s JOIN r`: E4001 unsupported, as planned.

Readable-style FormatAST of INSERT ... SELECT does not re-parse (`...FROM s;\nRETURNING id;`), in

#610 race gate: PASS. Ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) in /Users/ajitpratapsingh/dev/GoSQLX-worktrees/fix-insert-grammar-gaps at HEAD f59b1a8. 44 packages reported ok, with no FAIL lines and no data races; exit status 0. The pool tests (pool_reset_test, pool_drawn_policy_test) and all new #610 tests passed in that run. Log: /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/610/race.txt

Breaking Changes

parser: INSERT gaps: RETURNING after INSERT ... SELECT, DEFAULT VALUES, MySQL INSERT ... SET, INSERT INTO t (SELECT ...) #610: The only exported API change is additive: two new bool fields on ast.InsertStatement.
DefaultValues is true for INSERT INTO t DEFAULT VALUES.
SetSyntax marks the MySQL/MariaDB INSERT ... SET form, which is stored as Columns plus one Values row.
Both are reset in the pool release path. No exported signatures change and no new exported functions are added.

Observable behaviour changes:

These forms now parse: RETURNING after INSERT ... SELECT, INSERT ... SELECT RETURNING, DEFAULT VALUES (all dialects), INSERT ... SET (MySQL/MariaDB), parenthesized INSERT query sources (all dialects), and UPDATE ... FROM with comma-separated tables (all dialects).
INSERT RETURNING is now matched case-insensitively (lowercase returning works).
Inside an INSERT source or UPDATE FROM list, an unquoted RETURNING is no longer an implicit alias.
UPDATE ... FROM ... JOIN returns an unsupported-feature error.
SQL() and FormatAST render DEFAULT VALUES and the SET form. Parenthesized sources render without their parentheses, which is semantically equivalent.

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 #610
Follow-up to #566

🤖 Generated with Claude Code

https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

vercel · 2026-10-14T05:30:14Z

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

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

github-actions · 2026-10-14T05:33:54Z

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

Review of PR #623 (INSERT grammar gaps, #610)

I read the full diff of the parser, AST and formatter changes. I did not run the test suite, and I did not read the new test files in detail. Overall the change is well scoped, and the dmlReturning flag is saved and restored correctly (it is restored before the error check, so error paths are covered). Below are some non-blocking observations.

Correctness / edge cases

Flag scope is broader than the INSERT source's top level (dml_insert.go parseInsertSourceQuery, parser.go isDMLReturningKeyword). dmlReturning stays true for the whole source query, including nested subqueries and the select list. For example, INSERT INTO t SELECT x returning FROM s or ... FROM (SELECT 1) returning now fail where an alias used to work. Real queries rarely hit this, but the behaviour change goes slightly beyond the "no implicit alias before the trailing RETURNING" intent. A cheap hardening is to clear the flag when entering parenthesized subqueries and expression subqueries. Another option is to apply it only in parseFromTableReference and parseFromClause, and not in canBeAlias for select-list columns.
INSERT ... SET only accepts a bare identifier as the column (dml_insert.go, the SET case). MySQL also allows qualified names (SET t.a = 1), and a = DEFAULT depends on parseExpression handling DEFAULT. Please confirm those cases either work or return a clear error. Tests for them would help.
ON DUPLICATE KEY UPDATE after SET is probably handled, because the common tail code runs after the switch. The PR notes that its rendering is deferred. A round-trip test INSERT INTO t SET a=1 ON DUPLICATE KEY UPDATE a=2 would lock this in.
UPDATE ... FROM is accepted in every dialect, including MySQL, where it is invalid. This matches the PR's stated choice of lenient parsing, but it is worth a doc note. If dialect gating is wanted later, p.Capabilities() is the natural place for it.
Parenthesized sources lose their parentheses on render. This is equivalent for a single SELECT. It is not equivalent for forms like (SELECT ... LIMIT 1) combined with set operations, which parser: parenthesized set operations not accepted as derived tables or statements; ((SELECT ...)) t gives a misleading join-group error #612 defers. Make sure parser: parenthesized set operations not accepted as derived tables or statements; ((SELECT ...)) t gives a misleading join-group error #612 stays open, so the silent AST flattening doesn't get forgotten.
isInsertQueryParenStart treats (( as the start of a query source. INSERT INTO t ((a)) is not a valid column list anyway, so this is fine. It is worth a negative test that the resulting error message is sensible.

Code quality

isSetForm() in ast/sql.go and the inline condition in formatter/render.go duplicate the same predicate. Exporting a method or sharing one helper would stop the two from drifting apart.
The long !p.isX() && ... chains in select_subquery.go are duplicated in two places. This is pre-existing, but each new flag makes it worse. A small canBeTableAlias() helper would help.
The new DefaultValues and SetSyntax fields are reset in the pool release path, and Reset() clears dmlReturning. Both are good.

Performance: The change adds a bool check in front of a string comparison, so hot-path cost is negligible.

Security: I found no concerns. There is no new unbounded recursion, and the paren loop in parseInsertSourceQuery is bounded by the existing token stream and recursion limits. It would still be worth confirming that deeply nested ((((... input is covered by the recent recursion-depth bound from #597. The paren loop is iterative, but parseSubquery is not.

Tests: Coverage looks thorough (parent-commit failures were verified, plus fixpoint and error cases, flag-restore and pool-reset tests). I suggest adding the cases from items 1–3 above and a deeply nested paren case.

No blocking issues. The change looks good to merge once you've decided on item 1.

…DEFAULT VALUES, MySQL INSERT ... SET, parenthesized INSERT sources and UPDATE ... FROM (#610) Several common INSERT forms were rejected: - RETURNING after an INSERT ... SELECT source: the implicit table-alias check took the unquoted RETURNING (tokenized as IDENTIFIER) as the alias of the last FROM/JOIN table, and the INSERT RETURNING check was case-sensitive. A parser context flag (dmlReturning), set only while parsing an INSERT source query or an UPDATE ... FROM list, now stops an unquoted RETURNING from becoming an implicit table or select-item alias and lets a FROM-less SELECT end before it. Outside those contexts `SELECT * FROM s returning` still yields alias "returning". The flag is restored before error checks and cleared by Reset. - INSERT INTO t DEFAULT VALUES: new InsertStatement.DefaultValues. - MySQL/MariaDB INSERT INTO t SET col = expr, ...: new InsertStatement.SetSyntax; assignments are stored as Columns plus one Values row so linter, extraction, security and span code are unchanged. SQL() and pkg/formatter emit SET when the shapes match and fall back to the VALUES form otherwise. - INSERT INTO t [(cols)] (SELECT ...): a '(' followed by SELECT, WITH or '(' is a query source, unwrapped iteratively and rendered without the parentheses. - PostgreSQL/SQL Server UPDATE t SET ... FROM s [, ...] now populates UpdateStatement.From; JOIN inside UPDATE ... FROM returns an unsupported-feature error because the AST has no join list. Both new InsertStatement fields are reset in releaseInsertStatement. All reported inputs failed with the issue's E2002 errors before the change; the new tests check AST shape and a SQL() re-parse fixpoint per dialect, plus FormatAST round-trips (readable style for nested query sources is left to #602). Fixes #610 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

F1 (parser): isReturningKeyword treated any identifier spelled RETURNING as the keyword, including backtick- and bracket-quoted ones. Inside an INSERT source or UPDATE ... FROM list, a quoted alias such as `returning` or [returning] was read as the start of the RETURNING clause, so valid SQL that parsed before #610 failed (E2002/E2004). The identifier branch now requires an unquoted token: Token.Quote must be zero (bracket identifiers set '['), and because the tokenizer leaves Quote unset for backtick identifiers, a source span wider than the value also counts as quoted. TestDMLReturningQuotedAlias covers table, join and nested-subquery aliases in INSERT ... SELECT (mysql, mariadb, sqlserver, postgresql) and UPDATE ... FROM (sqlserver, postgresql). F2 (cli): the gosqlx format renderer (cmd/gosqlx/cmd/sql_formatter.go) ignored the new UpdateStatement.From and InsertStatement.DefaultValues fields, so `format -i` rewrote UPDATE ... FROM without its source list (changing meaning) and INSERT ... DEFAULT VALUES as the invalid "INSERT INTO t", exiting 0. formatUpdate now renders the FROM list and formatInsert renders DEFAULT VALUES. TestFormatCommand_InsertUpdateForms checks exact output, readable layout, and that re-parsed output has the same SQL() as the input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

F3: the RETURNING stop used while parsing an INSERT source query or UPDATE ... FROM list leaked into nested scopes, so inputs such as "INSERT INTO t WITH c AS (SELECT * FROM s returning) SELECT * FROM c", "... WHERE a IN (SELECT b FROM r returning)" and "... FROM (SELECT a FROM r) returning" regressed from parsing to failing in dialects that do not reserve RETURNING. parseSubquery, derived-table parsing and CTE bodies now clear and restore p.dmlReturning, and the INSERT source calls a non-clearing parseQuerySource. RETURNING followed by a token that cannot start a RETURNING list (ON, USING, comma, dot, ")", join/clause keywords, end of statement) is treated as an alias, which fixes "FROM s JOIN r returning ON s.id = returning.id". F5: pkg/schema validateUpdate ignored UpdateStatement.From, producing false "not referenced in FROM clause" errors for valid UPDATE ... FROM and never checking that FROM tables exist. It now checks each named FROM table and adds its name/alias to the table map, mirroring validateSelect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

- val-0: isDMLReturningKeyword now treats a bare `returning` in an INSERT source FROM list or UPDATE ... FROM list as an alias when the next token is any join keyword (models.TokenType.IsJoinKeyword: LEFT, RIGHT, OUTER, ON, USING, ...), OFFSET, FETCH, FOR or WITH (SQL Server table hints), or starts a dialect-specific clause that may follow a table reference (WINDOW, Snowflake QUALIFY, PIVOT/UNPIVOT, Oracle/MariaDB START WITH and CONNECT BY, SAMPLE/TABLESAMPLE, MATCH_RECOGNIZE, time travel, MINUS, ClickHouse FINAL/PREWHERE/ARRAY JOIN/SETTINGS). The new nextStartsTableClause helper reuses the existing dialect-gated FROM-alias predicates one token ahead and restores the parser position. Inputs such as `INSERT INTO t SELECT * FROM s returning LEFT JOIN r ON ...` parse again as they did before RETURNING support; regression cases were added to TestDMLReturningNestedScopes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

github-actions · 2026-10-14T06:38:16Z

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

Review of PR #623 (INSERT grammar gaps, #610)

I read the parser diff in full: parser.go, dml_insert.go, dml_update.go, cte.go, expressions_literal.go, select_*.go. I did not run the test suite and did not read the new test files line by line.

Overall: The change is well scoped and I found no blocking issues.

Earlier review's item 1 is already handled.

parseSubquery, parseDerivedTableSelect and parseCommonTableExpr all clear dmlReturning and restore it with defer.
parseInsertSourceQuery and the UPDATE FROM path restore it on the error path too.
Reset() clears the flag.

Non-blocking observations

Select-list alias edge case.
- In isDMLReturningKeyword, a following FROM is not in the "not a RETURNING clause" list.
- So INSERT INTO t SELECT x returning FROM s still treats returning as the clause keyword and errors, where it used to be an alias.
- Adding models.TokenTypeFrom to that switch would fix it. A RETURNING list can never start with FROM.
- It is worth a test.
INSERT ... SET columns are bare identifiers only (dml_insert.go).
- SET t.a = 1 returns "expected column name".
- That is a clear error, but MySQL accepts the qualified form.
- A test that pins the current behavior, plus one for a = DEFAULT and ... SET a=1 ON DUPLICATE KEY UPDATE a=2, would be useful.
isQuotedIdentifierToken relies on a span-width heuristic. End.Column - Start.Column > len(Value) is correct for backtick identifiers today. It would silently break if the tokenizer ever changed how it reports spans, for example with multi-byte values or escaped quotes. A comment pointing at the tokenizer behavior, or a test covering a backtick `returning` alias, would pin it down.
INSERT INTO t ((a)).
- isInsertQueryParenStart treats (( as a query start, so the error comes from parseQuerySource, not from a column-list error.
- The message is probably fine, but a negative test would confirm it reads sensibly.
- The paren-consuming loop in parseInsertSourceQuery is iterative and bounded by the token stream, so there is no recursion risk there.
Parenthesized sources are flattened on render. This is equivalent for a single SELECT, but not for (SELECT … LIMIT 1) UNION …. That case is deferred to parser: parenthesized set operations not accepted as derived tables or statements; ((SELECT ...)) t gives a misleading join-group error #612, so keep that issue open.
Duplicated predicate.
- isSetForm() in ast/sql.go and the inline condition in formatter/render.go encode the same predicate.
- The long !p.isX() && … alias guards are now duplicated in three places (parseFromTableReference, parseJoinedTableRef, canBeAlias).
- A small shared helper would stop them drifting apart.
UPDATE … FROM is accepted in every dialect, including MySQL, where it is invalid. That is consistent with the stated lenient approach. A line in the docs, or later gating via p.Capabilities(), would cover it.

Performance: The hot-path cost is one bool check ahead of existing comparisons. nextStartsTableClause runs only when dmlReturning is set and the token is RETURNING, so it is negligible.

Security: No new unbounded recursion, and pool reset covers the new AST fields.

Tests: Coverage looks thorough. It includes parent-commit failure verification, fixpoint round-trips, flag-restore, pool-reset and error cases. The only gaps are the edge cases in items 1–4.

This looks good to merge. I'd consider item 1, a one-line fix, before merging.

ajitpratap0 · 2026-10-14T06:38:17Z

Merge train (batch 3): rebased onto main (4273dbb). This PR got two rounds of adversarial fixes.

Must-fix, fixed in 933c646 (Opus re-verified):

A quoted identifier named returning was treated as the RETURNING keyword.
gosqlx format dropped UPDATE … FROM and DEFAULT VALUES.

Should-fix, fixed in 5710b92 and 7e9e7e2 (Opus validated after a second round):

dmlReturning is now saved and restored around nested scopes: derived tables, IN/EXISTS, CTE bodies and scalar subqueries. So ... FROM (SELECT 1) returning keeps returning as an alias again.
Join and clause keywords after a returning alias (LEFT/RIGHT/OUTER JOIN, OFFSET, FETCH, FOR, WINDOW, dialect clause starters) no longer start a RETURNING clause.
pkg/schema validateUpdate now includes tables from UPDATE … FROM.

Bot review items, checked on HEAD:

A bare returning alias in the top-level select list of an INSERT source is still read as RETURNING. This is intended; RETURNING is reserved in PostgreSQL.
SET t.a = 1 gives "E2002 expected =, got PERIOD" and SET a = DEFAULT gives E2001. Both are clear errors; support for these goes to a follow-up issue.
INSERT … SET … ON DUPLICATE KEY UPDATE parses. The legacy CLI renderer drops the ODKU tail, the same pre-existing gap as INSERT … VALUES … ON DUPLICATE KEY UPDATE on main. It is fixed by fix(formatter,ast,parser): no inner semicolons; keep IGNORE NULLS, named args, ON DUPLICATE KEY, OUTPUT, SETTINGS (#602, #600) #631 (ast/formatter: SQL() and pkg/formatter drop IGNORE NULLS, named arguments, ON DUPLICATE KEY UPDATE and INSERT OUTPUT; parser discards SELECT ... SETTINGS #600, ODKU rendering) and fix(cli): render gosqlx format through pkg/formatter and refuse non-round-tripping output (#599) #624 (cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599, CLI on pkg/formatter, which refuses output that doesn't round-trip), both later in this train.
Deep nesting: 300 levels of ((((SELECT 1)))) as the INSERT source parse fine, because the paren loop is iterative.

…sts with #632/#623 Integration with #632 (now on main), which quotes reserved-word names on render: - ALTER TABLE ... ADD COLUMN "order" now formats correctly, so the guard test expects the quoted output instead of a refusal. - In MySQL/MariaDB a backtick keyword such as `select` would render as "select", which MySQL reads as a string unless ANSI_QUOTES is set. The new checkMySQLDoubleQuotes refuses output that adds double-quoted tokens the input did not have, keeping the CLI's previous safe refusal. Integration with #623: pkg/formatter writes table aliases without AS, so the UPDATE ... FROM list expectation becomes "u v". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

…ound-tripping output (#599) (#624) * fix(cli): render gosqlx format through pkg/formatter and refuse non-round-tripping output (#599) `gosqlx format` used its own 1,476-line renderer in cmd/gosqlx/cmd/sql_formatter.go. Any expression type it did not handle fell back to TokenLiteral(), and formatInsert dropped every clause after VALUES/SELECT. The command exited 0 with SQL of a different meaning, or with SQL that does not parse (CAST -> `CAST`, `a = ANY(...)` -> `ANY`, tuples -> `TUPLE`, `arr[1]` -> `[]`, ROLLUP/CUBE/GROUPING SETS, WITHIN GROUP, `EXISTS (SELECT 1 UNION SELECT 2)` -> `EXISTS ()`, ON CONFLICT/RETURNING dropped, PIVOT dropped, lossy ALTER TABLE). - sql_formatter.go is now a thin adapter. SQLFormatter, NewSQLFormatter, FormatterOptions and Format keep their signatures. Each statement is rendered with pkg/formatter.FormatStatement, with indent, compact and keyword case mapped to ast.FormatOptions. AddSemicolon is off and the CLI keeps its "; " / ";\n\n" separators. FormatterOptions gains an additive Dialect field. AlignColumns is retained but ignored. All parallel rendering code is deleted. - formatSQL now verifies its output. It re-parses the rendering with the same dialect and requires the same statement count and a case-insensitive equal SQL(). Otherwise it returns a "formatted output does not re-parse / changes the query" error, so stdout, -o and -i never receive corrupted SQL and --check counts the file as failed. Lowercase-keyword output is verified via its uppercase twin, because the parser rejects some lowercase keyword sequences. - pkg/formatter renders *ast.AlterStatement ALTER TABLE operations (ADD COLUMN, ADD CONSTRAINT, DROP COLUMN/CONSTRAINT [CASCADE], RENAME TO, RENAME COLUMN, ALTER COLUMN) in a new render_alter.go. Before, it rendered a bare "ALTER". Non-table ALTER keeps its previous output. render.go gains one switch case. Tests: - TestFormatSQL_Issue599 covers every reproduction in the issue (compact output must match exactly; readable output must round-trip and be idempotent). Rows that pkg/formatter itself still loses skip with the blocking issue and self-heal when the fix lands. - TestFormatSQL_CorpusRoundTrip formats all 134 parseable testdata/**/*.sql files in three modes, re-parses them, compares SQL() and checks idempotence. 8 documented known gaps may error but never emit changed SQL. - TestFormatStatement_AlterTable* cover the new ALTER rendering. Blocked on library fixes (these now match pkg/formatter and are fixed automatically when they land): - #600: named args, IGNORE NULLS, SETTINGS, ON DUPLICATE KEY UPDATE, OUTPUT - #603: JOIN USING - #604: WITH (NOLOCK) - #601: NOT EXISTS (now an explicit error) - #611: quoted aliases and identifiers (now an explicit error) Follow-ups to file: - CROSS/OUTER APPLY rendering - MATCH ... AGAINST rendering - Oracle CONNECT BY rendering - Snowflake QUALIFY rendering - backslash literals in dialects that keep them literal - ALTER ROLE/POLICY/CONNECTOR rendering - --no-uppercase flag has no effect - parser case-sensitivity for lowercase `returning` / `cross join lateral` CLI behaviour change: readable output layout now follows pkg/formatter, and output that would not round-trip is now an error with a non-zero exit instead of exit 0. Fixes #599 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve * fix(cli): drop duplicated "formatting failed" prefix in gosqlx format errors (#599) formatSQL wrapped its errors with "formatting failed:" and both format command paths wrapped them again, so users saw "formatting failed: formatting failed: ...". formatSQL now uses step-specific wrapping and returns the self-describing round-trip errors unchanged; the command keeps the single prefix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve * fix(cli): address adversarial review for #624 F1 (must-fix, regression): gosqlx format rewrote a non-executing EXPLAIN into executable SQL. The parser splits "EXPLAIN VERBOSE|ANALYZE <stmt>" into DescribeStatement{TableName: "VERBOSE"} plus <stmt>; the old CLI renderer refused DescribeStatement, but rendering through pkg/formatter accepted it, and the SQL() round-trip check is equally lossy, so `format -i` turned "EXPLAIN VERBOSE DELETE FROM users;" into "DESCRIBE VERBOSE; DELETE FROM users" with exit 0 (same for UPDATE, DROP, TRUNCATE, INSERT, EXPLAIN ANALYZE and MySQL EXPLAIN ANALYZE). formatSQL now refuses, before rendering, DescribeStatement (drops EXPLAIN options, the explained query and DESCRIBE object kinds) and UnsupportedStatement (rendered as a comment), restoring the base "unsupported statement type" error and leaving input unchanged. As a general guard it also refuses any parse with more statements than semicolon-separated segments in the input, since a statement split by the parser cannot be detected by re-parsing the output. The underlying parser bug (EXPLAIN VERBOSE/ANALYZE splitting into two statements, DescribeStatement losing options and object kind) still needs its own issue. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve * fix(cli): address adversarial review follow-ups for #624 F2: the format round-trip check passed trivially for statements without a SQL() method (ALTER, sequences), since AST.SQL() skips them and "" equals "". verifyFormattedSQL now also compares a canonical pkg/formatter rendering of every statement, and compares the quoted tokens of the input and output: each quoted identifier must stay quoted or mean the same identifier bare (ASCII word, not a keyword for the dialect, and unchanged by the dialect's case folding), so `ADD COLUMN "order"`, "Mixed" and MySQL `select` are refused instead of written bare with exit 0. This also refuses SELECT "Mixed" and corpus files gorm_joins.sql (alias "CompanyName"), oracle/14_pivot.sql and 15_unpivot.sql (PIVOT IN 'North' rendered as the identifier North), which were previously written changed; they are listed as known gaps. F3 (guard only): every input string literal value must appear in the output, so backslash literals in dialects where backslash is not an escape are refused with an explicit "changes string literal" error. The renderer still re-escapes backslashes MySQL-style; making LiteralValue rendering dialect-aware needs the dialect threaded into pkg/sql/ast SQL() and is left as a follow-up. F5: UnaryExpression.SQL() wrote nested unary minus as "--", which starts a line comment; it now writes "- -1". Tests in pkg/sql/ast and pkg/formatter. F6: DESCRIBE/DESC were already refused by checkFormattable (previous commit); added Snowflake DESCRIBE STAGE / VIEW / DESC TABLE regression tests. F7: doc.go, formatter.go and sql_formatter.go comments now describe the check as a comparison of the input's parse with the output's parse, and state that what the parser drops (comments, skipped clauses) is not detected. F9: format --check no longer prints "All N files are properly formatted" when some files failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve * test(cli): expect pkg/formatter readable layout in JOIN USING format test TestFormatCommand_JoinUsing_Issue603 (#603) pinned the old CLI renderer's readable layout ("SELECT\n *\nFROM ..."). #599 routes `gosqlx format` through pkg/formatter, whose readable layout keeps a single-item select list on the SELECT line ("SELECT *\nFROM ..."). Only the readable expectation changes; the USING assertions (no "ON" rendering, re-parsed Using columns, nil Condition) are unchanged. Integration fix for #603 x #599; port into whichever of the two lands second. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve * test(cli): drop corpus known-gap entries fixed by #601 With #601 (NOT EXISTS renders as a prefix NOT) and #599 (gosqlx format via pkg/formatter) both applied, corpus/tpch/q21_suppliers_kept_waiting.sql and mysql/19_not_exists.sql round-trip in every mode, so they no longer belong in corpusFormatKnownGaps. Keeping them would let a regression of The ent_queries.sql reason now names what is still missing after #611: the quoted qualifier in "groups"."id" (Identifier.Table quoting was deferred by #611). Integration fix for #599 x #601/#611; port into whichever lands second. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve * fix(cli): refuse MySQL output with added ANSI double quotes; align tests with #632/#623 Integration with #632 (now on main), which quotes reserved-word names on render: - ALTER TABLE ... ADD COLUMN "order" now formats correctly, so the guard test expects the quoted output instead of a refusal. - In MySQL/MariaDB a backtick keyword such as `select` would render as "select", which MySQL reads as a string unless ANSI_QUOTES is set. The new checkMySQLDoubleQuotes refuses output that adds double-quoted tokens the input did not have, keeping the CLI's previous safe refusal. Integration with #623: pkg/formatter writes table aliases without AS, so the UPDATE ... FROM list expectation becomes "u v". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve * fix(cli): require MySQL output to keep the input's double-quoted tokens Review follow-ups for the rebase integration: - checkMySQLDoubleQuotes compared only counts, so a double-quoted MySQL string written bare could cancel out an identifier newly written in ANSI quotes. It now requires the output to keep exactly the input's double-quoted tokens, which also refuses WHERE name = "bob" -> name = bob (a string silently becoming a column reference). Input that uses "..." as a name where MySQL cannot accept a string ("users"."id", FROM "users") is ANSI_QUOTES-style and is not checked; no MySQL/MariaDB corpus file is refused. - TestSQLFormatter_NotExistsDoesNotPanic: the #601 Skipf is unreachable now that #632 is on main; fail instead. - The corpus round-trip test now fails when a known-gap file starts to format, so the list cannot go stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

github-actions Bot added parser ast AST node changes labels Oct 14, 2026

ajitpratap0 changed the title ~~parser: INSERT gaps: RETURNING after INSERT ... SELECT, DEFAULT VALUES, MySQL INSERT ... SET, INSERT INTO t (SELECT ...) (#610)~~ fix(parser): INSERT ... SELECT RETURNING, DEFAULT VALUES, MySQL INSERT ... SET and parenthesized sources (#610) Oct 14, 2026

vercel Bot deployed to Preview October 14, 2026 05:34

View deployment

ajitpratap0 and others added 4 commits October 14, 2026 12:07

ajitpratap0 force-pushed the fix/insert-grammar-gaps branch from f59b1a8 to 4273dbb Compare October 14, 2026 06:37

github-actions Bot added the cli label Oct 14, 2026

vercel Bot deployed to Preview October 14, 2026 06:39

View deployment

ajitpratap0 merged commit 3baad9d into main Oct 14, 2026
28 checks passed

ajitpratap0 mentioned this pull request Oct 14, 2026

perf: repo-wide optimization audit (2026-10-11) — tracking issue #566

Open

22 tasks

Navigation Menu

Repository navigation

fix(parser): INSERT ... SELECT RETURNING, DEFAULT VALUES, MySQL INSERT ... SET and parenthesized sources (#610) - #623

fix(parser): INSERT ... SELECT RETURNING, DEFAULT VALUES, MySQL INSERT ... SET and parenthesized sources (#610)#623
ajitpratap0 merged 4 commits into
mainfrom
fix/insert-grammar-gaps

ajitpratap0 commented Oct 14, 2026

Uh oh!

vercel Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

ajitpratap0 commented Oct 14, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ajitpratap0 commented Oct 14, 2026

Summary

#610 parser: INSERT gaps: RETURNING after INSERT ... SELECT, DEFAULT VALUES, MySQL INSERT ... SET, INSERT INTO t (SELECT ...)

Test Plan

Breaking Changes

Notes

Uh oh!

vercel Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review of PR #623 (INSERT grammar gaps, #610)

Uh oh!

github-actions Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review of PR #623 (INSERT grammar gaps, #610)

Uh oh!

ajitpratap0 commented Oct 14, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Oct 14, 2026 •

edited

Loading

github-actions Bot commented Oct 14, 2026 •

edited

Loading

github-actions Bot commented Oct 14, 2026 •

edited

Loading