Skip to content

Commit 2249e14

Browse files
kyleconroyclaude
andauthored
endtoend: run sqlc fmt over the sqlite testdata queries (#4582)
* fmt: keep the author's operator, join and parameter spellings; lowercase coalesce SQLite gives several constructs more than one spelling, and the formatter was silently picking one: != printed as <> and == as =, a comma-separated FROM item printed as JOIN, a bare JOIN with no ON printed as CROSS JOIN — a planner hint in SQLite the author did not write — and a numbered parameter printed as a bare ?, which is worse than a spelling change: reordered ?N parameters bind by their numbers, so VALUES (?2, ?1) rewritten to (?, ?) swaps its arguments. Operators keep pg_query's shape: A_Expr.Name is the operator as the engine's parser saw it, and since meyer's tree keeps only the operator kind, the sqlite converter reads the author's spelling back out of the source between the operands. The compiler already recognizes every spelling, as it must for MySQL, whose canonical != flows through the same lists. Joins that SQLite treats distinctly become distinct: JoinType gains JoinTypeCross (the planner hint) and JoinTypeComma (its own syntax) beyond the libpg_query set, the sqlite converter maps to them, and the printer spells each as itself — which retires the printer's guess that an inner join with no condition must be a CROSS JOIN. PostgreSQL, whose grammar really does mean CROSS JOIN by that shape (a bare JOIN without ON is a syntax error there), now says so in its converter. Redundant spellings still normalize: INNER JOIN prints as JOIN and LEFT OUTER JOIN as LEFT JOIN, which mean exactly the same thing. Parameters use the numbering the node already records: Dialect.Param gains a numbered flag, ParamRef passes its Dollar field, and sqlite prints ?N for a numbered parameter and ? for a bare one. Compound selects gain the seam boundary the clauses already had: an author who broke the line around UNION, INTERSECT or EXCEPT keeps the operator on its own line, and a one-line compound stays on one line. Statements sqlc has no node for (PRAGMA and friends) stay in the file: ParseFile kept them out of its statement list, so the formatter never saw their extents — it deleted the statements and pulled the name annotations of their neighbours inside the preceding query. They now stay in the list as TODOs, which render as nothing and fall back verbatim; Parse filters them for the compiler, whose skip behavior is unchanged. The file-level belt also refuses any result that changes the file's statement count, so nothing of this class can slip through again. The ON CONFLICT DO UPDATE SET list also gains the boundaries the UPDATE statement's own SET list has: an author who broke the assignments keeps one per line, with the conflict clause's WHERE at clause level, and a one-line upsert stays on one line. COALESCE also drops to lower case: it printed upper-case only because sqlc special-cases it into a dedicated node for nullability inference whose Format hardcoded the spelling, while every other function call prints through FuncCall with its identifier folded lower. The fmt endtoend case pins all of it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MTvpHqNMadH12pTtsgUq2 * endtoend: run sqlc fmt over the sqlite testdata queries Format every sqlite query file in the end-to-end corpus with the new formatter and regenerate the affected goldens (the generated code embeds the query text). The fmt case's own input stays unformatted — it is the formatter's fixture — and nine files are left as written because they only parse after the compiler's preprocessing (sqlc.arg/narg/slice/ embed, @nAmed parameters). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MTvpHqNMadH12pTtsgUq2 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f9b7b21 commit 2249e14

115 files changed

Lines changed: 774 additions & 619 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

internal/cmd/fmt.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,12 @@ func Format(ctx context.Context, dir, filename string, o *Options) (map[string]s
188188
continue
189189
}
190190
// File-level belt for the reprinter path: no formatting result may
191-
// change the file's comments. A violation is a formatter bug; keep
192-
// the file as written and say so.
191+
// change the file's comments or its statement count. A violation is
192+
// a formatter bug; keep the file as written and say so.
193193
before, err1 := f.ParseFile(strings.NewReader(string(contents)))
194194
after, err2 := f.ParseFile(strings.NewReader(formatted))
195-
if err1 == nil && (err2 != nil || !sameComments(before.Comments, after.Comments)) {
196-
fmt.Fprintf(stderr, "%s: skipped: formatting would alter comments (this is a bug in sqlc fmt)\n", rel)
195+
if err1 == nil && (err2 != nil || len(before.Stmts) != len(after.Stmts) || !sameComments(before.Comments, after.Comments)) {
196+
fmt.Fprintf(stderr, "%s: skipped: formatting would alter the file (this is a bug in sqlc fmt)\n", rel)
197197
continue
198198
}
199199
output[file] = formatted

internal/compiler/output_columns.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ func isTableRequired(n ast.Node, col *Column, prior int) int {
453453
return helper(tableOptional, tableRequired)
454454
case ast.JoinTypeFull:
455455
return helper(tableOptional, tableOptional)
456-
case ast.JoinTypeInner:
456+
case ast.JoinTypeInner, ast.JoinTypeCross, ast.JoinTypeComma:
457457
return helper(tableRequired, tableRequired)
458458
}
459459
case *ast.List:

internal/endtoend/testdata/between_args/sqlite/go/query.sql.go

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
-- name: GetBetweenPrices :many
2-
SELECT *
3-
FROM products
4-
WHERE price BETWEEN ? AND ?;
2+
SELECT *
3+
FROM products
4+
WHERE price BETWEEN ? AND ?;
55

66
-- name: GetBetweenPricesTable :many
7-
SELECT *
8-
FROM products
9-
WHERE products.price BETWEEN ? AND ?;
7+
SELECT *
8+
FROM products
9+
WHERE products.price BETWEEN ? AND ?;
1010

1111
-- name: GetBetweenPricesTableAlias :many
12-
SELECT *
13-
FROM products as p
14-
WHERE p.price BETWEEN ? AND ?;
12+
SELECT *
13+
FROM products AS p
14+
WHERE p.price BETWEEN ? AND ?;

internal/endtoend/testdata/builtins/sqlite/go/mathfunc.sql.go

Lines changed: 29 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)