Fix sqlc fmt corruption in PostgreSQL and MySQL formatters - #4596
Open
kyleconroy wants to merge 1 commit into
Open
Fix sqlc fmt corruption in PostgreSQL and MySQL formatters#4596kyleconroy wants to merge 1 commit into
kyleconroy wants to merge 1 commit into
Conversation
Fix six root causes of corruption found by running sqlc fmt across the endtoend test corpus and verifying the results: PostgreSQL: a named parameter with a cast (@name::type) printed as '@ name::type' because the cast binds tighter than the @ operator, so the printer saw a prefix operator and added a space — silently dropping the parameter from generated code. Prefix sigils now glue to operands that start with an identifier. MySQL (dolphin): the formatter reprinted from the compiler's normalized AST, which loses information the compiler doesn't need but a formatter must keep. sqlc fmt now uses a format parser that preserves identifier case (table names are case-sensitive on most servers), and the converter keeps what it used to drop: - NULL, TRUE/FALSE, and decimal literals convert by datum kind, so NULL no longer prints as '', 1.0 as 0, or true as 1. This also fixes type inference: a bool literal column is now bool, not int32 (selectstatic golden regenerated). - SELECT DISTINCT keeps its DISTINCT. - The ORDER BY of a compound (UNION) statement survives. - GROUP_CONCAT(... ORDER BY ...) keeps its ordering. - Optimizer hints (/*+ ... */) are carried through to printing. - Multi-table UPDATE keeps its JOIN's ON condition and the table qualifiers in SET. - DECIMAL(p,s) precision and UNSIGNED survive in column definitions. Backstop: dolphin now implements Fingerprint (via marino's restore with identifier case preserved and redundant parens unwrapped), giving MySQL the same proof PostgreSQL has: fmt accepts a formatted statement only when it provably means what the author wrote, and otherwise leaves the statement exactly as written. That is what fixes SHOW WARNINGS, which the parser rewrites into a synthetic SELECT for analysis: it now falls back to the original text instead of printing the internal form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZBvADuHMZEjoF551NccgP
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
注册 for free
to join this conversation on GitHub.
Already have an account?
登录 to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running
sqlc fmtacross all 800 PostgreSQL/MySQL endtoend configs and verifying each result (regenerated code compared with embedded SQL stripped, plus a parse-tree equivalence check for MySQL) surfaced 21 cases where formatting corrupted queries. They reduce to six root causes, all fixed here.PostgreSQL
@name::typeprinted as@ name::type. The cast binds tighter than@, so the printer saw a prefix operator and added a space — and sqlc's named-param scanner then no longer matched, silently dropping the parameter from generated code. Prefix sigils (@,:,$) now glue to operands that start with an identifier. The pg fingerprint could never catch this one (@nameand@ nameparse to the same tree), so the printer fix is the fix. Affected corpus cases:named_param,unnest,unnest_star.MySQL (dolphin)
The formatter reprinted from the compiler's normalized AST, which loses information the compiler doesn't need but a formatter must keep:
`Event`becameevent, renaming the table on servers where table names are case-sensitive.sqlc fmtnow usesdolphin.新建FormatParser(), which preserves the author's identifier case; the compiler's parser keeps lowercasing for case-insensitive catalog matching, so analysis is unchanged.NULLprinted as'',1.0as0,trueas1. Literals now convert by datum kind (ast.Null,ast.Boolean, decimals keep their written scale). This also fixes type inference: a bool literal column is nowbool, notint32(selectstatic/mysqlgolden regenerated to match PostgreSQL's inference).SELECT DISTINCTlost itsDISTINCT; aUNION's trailingORDER BYvanished;GROUP_CONCAT(... ORDER BY ...)lost its ordering; optimizer hints (/*+ ... */) were deleted; and a multi-tableUPDATE ... JOINlost itsONcondition andSETqualifiers — printing an update of the whole cross product. All preserved now, via converter fixes plus two print-fidelity AST fields (UpdateStmt.TableRefs,ResTarget.Relation) that analysis ignores.DECIMAL(10,5)printed asDECIMALandUNSIGNEDwas dropped in column definitions.SHOW WARNINGSrewritten — dolphin convertsSHOWinto a syntheticSELECTfor analysis, and the printer printed that internal form.新建 safety net: a MySQL fingerprint. dolphin now implements
Fingerprint(the prooffmtalready requires from PostgreSQL via pg_query): the original and formatted statements are parsed with marino and their canonical restore forms compared, with identifier case preserved and redundant parentheses unwrapped. Any statement the printer cannot provably reproduce —SHOW WARNINGS, comma-join multi-tableUPDATEs — is left exactly as written, which isfmt's documented contract. This is what fixes theSHOWclass, and it structurally prevents this whole family of bugs from corrupting queries again.Testing
internal/endtoend/testdata/fmt/{mysql,postgresql}(goldens regenerated).--tags=examplessuite passes against live PostgreSQL 16 and MySQL 9.One pre-existing cosmetic limitation, unchanged here: a MySQL
CREATE TABLEin a query file collapses to one line, because marino exposes no column positions for the author-line-break machinery to anchor on.🤖 Generated with Claude Code
https://claude.ai/code/session_01UZBvADuHMZEjoF551NccgP
Generated by Claude Code