From 71a6e6f6c16c378b38f801f83237cf0716d7ba26 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 17:06:58 +0000 Subject: [PATCH] Upgrade to oliphant 0.2.0 and wire up sqlc fmt for PostgreSQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oliphant 0.2.0 adds parser.ParseFile, which returns the parse tree along with the comment tokens its scanner already produces. That is exactly what sqlc fmt needs from an engine, so the PostgreSQL parser now implements ParseFile — Parse delegates to it, keeping the statement-span conventions unchanged — and fmt formats postgresql query files. PostgreSQL also brings a proof the other engines cannot offer: oliphant's pg_query-compatible fingerprint. Where an engine implements Fingerprint, fmt only accepts a formatted statement whose fingerprint matches the original's, and falls back to the text as written otherwise. The fmt/postgresql end-to-end case now commits its formatted diff instead of an unsupported-engine notice. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01K8CZxjsrAjJsvGfZNrR7Gd --- go.mod | 2 +- go.sum | 4 +- internal/cmd/fmt.go | 29 ++++++++++- .../testdata/fmt/postgresql/stderr.txt | 1 - .../testdata/fmt/postgresql/stdout.txt | 48 +++++++++++++++++++ internal/engine/postgresql/parse.go | 45 +++++++++++++++-- internal/engine/postgresql/reserved.go | 7 +++ 7 files changed, 127 insertions(+), 9 deletions(-) delete mode 100644 internal/endtoend/testdata/fmt/postgresql/stderr.txt diff --git a/go.mod b/go.mod index 63ad23f388..311891d1f9 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/sqlc-dev/doubleclick v1.0.0 github.com/sqlc-dev/marino v0.3.0 github.com/sqlc-dev/meyer v0.1.2 - github.com/sqlc-dev/oliphant v0.1.0 + github.com/sqlc-dev/oliphant v0.2.0 github.com/sqlc-dev/teesql v1.1.0 github.com/sqlc-dev/zetajones v0.1.0 github.com/tetratelabs/wazero v1.12.0 diff --git a/go.sum b/go.sum index 53dd4e624d..6199ffde96 100644 --- a/go.sum +++ b/go.sum @@ -73,8 +73,8 @@ github.com/sqlc-dev/marino v0.3.0 h1:e9cinBXJFFa3yRpokYNNinWvkewgd6XVDgnlG0bOmTw github.com/sqlc-dev/marino v0.3.0/go.mod h1:mQxC2dgDE0DWHMb2B5jZNk7KToJuS6wnxnffBfYnq08= github.com/sqlc-dev/meyer v0.1.2 h1:40Ng9Glnx7CTf3yOYV7jfvDIN7voIFjrStkIULS+uws= github.com/sqlc-dev/meyer v0.1.2/go.mod h1:pS4USCRf/SLjWtaMcnTo4YrEEFKBj8CyyqlxcVUJQH8= -github.com/sqlc-dev/oliphant v0.1.0 h1:RAsO6BMitIzB2+swx/qzUR5nf6w4cQ1abgHIu+Fgppo= -github.com/sqlc-dev/oliphant v0.1.0/go.mod h1:fRM/t4FutRddTIq2YCuS4O9o+2rRwSwELRvLMqtPloo= +github.com/sqlc-dev/oliphant v0.2.0 h1:jJ/s2fh4Plj3U1HsdqiY/clw/IHb4FCNaHfqirwxcsI= +github.com/sqlc-dev/oliphant v0.2.0/go.mod h1:fRM/t4FutRddTIq2YCuS4O9o+2rRwSwELRvLMqtPloo= github.com/sqlc-dev/teesql v1.1.0 h1:3sVYQ9FGxQVcqrqQOQ27bk0aF4c4yN1H1zLL79uaSxQ= github.com/sqlc-dev/teesql v1.1.0/go.mod h1:WwOp9UtnxG17+eNFT5KXu/AGBQ8ucdGc8wIOpnj4XCI= github.com/sqlc-dev/zetajones v0.1.0 h1:VeG0atx6lNABr9V2bSI5vL9DvOKTHX0XjMqWUE/rv40= diff --git a/internal/cmd/fmt.go b/internal/cmd/fmt.go index 0b76e2e89d..3c77dbc22e 100644 --- a/internal/cmd/fmt.go +++ b/internal/cmd/fmt.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" "github.com/sqlc-dev/sqlc/internal/config" + "github.com/sqlc-dev/sqlc/internal/engine/postgresql" "github.com/sqlc-dev/sqlc/internal/engine/sqlite" "github.com/sqlc-dev/sqlc/internal/sql/ast" "github.com/sqlc-dev/sqlc/internal/sql/format" @@ -37,10 +38,13 @@ type queryFormatter interface { } // newQueryFormatter returns the formatter for engines fmt supports — -// SQLite today. An engine joins by teaching its parser to surface comments -// (meyer's ParseFile is the template) and adding its case here. +// SQLite and PostgreSQL today. An engine joins by teaching its parser to +// surface comments (meyer's and oliphant's ParseFile are the templates) and +// adding its case here. func newQueryFormatter(engine config.Engine) queryFormatter { switch engine { + case config.EnginePostgreSQL: + return postgresql.NewParser() case config.EngineSQLite: return sqlite.NewParser() default: @@ -374,16 +378,37 @@ func isCommentLine(line string) bool { } } +// fingerprinter is implemented by engines that can reduce a query to a +// fingerprint that survives changes in whitespace, case and layout — +// PostgreSQL via oliphant's pg_query-compatible Fingerprint. Where it is +// available, fmt gets a proof the other checks cannot give: the formatted +// statement still parses to the same query as the original. +type fingerprinter interface { + Fingerprint(string) (string, error) +} + // formatStmt returns the canonical form of a single statement, or the // original text when formatting cannot be proven to preserve the query. func formatStmt(f queryFormatter, raw *ast.RawStmt, orig string, interior []ast.Comment, src string) string { fallback := strings.TrimSuffix(strings.TrimSpace(orig), ";") + ";" if out, ok := formatWithComments(f, raw, interior, src); ok { + if fp, ok := f.(fingerprinter); ok && !sameFingerprint(fp, orig, out) { + return fallback + } return out } return fallback } +// sameFingerprint reports that both texts fingerprint successfully to the +// same value. Anything less is not a proof, so the caller keeps the +// statement as written. +func sameFingerprint(fp fingerprinter, orig, out string) bool { + a, err1 := fp.Fingerprint(orig) + b, err2 := fp.Fingerprint(out) + return err1 == nil && err2 == nil && a == b +} + // formatWithComments pretty-prints a statement with its interior comments // and proves the result faithful three ways before accepting it: every // comment survives (multiset equality after reparsing the output), the SQL diff --git a/internal/endtoend/testdata/fmt/postgresql/stderr.txt b/internal/endtoend/testdata/fmt/postgresql/stderr.txt deleted file mode 100644 index 4aa7068b83..0000000000 --- a/internal/endtoend/testdata/fmt/postgresql/stderr.txt +++ /dev/null @@ -1 +0,0 @@ -sqlc fmt does not yet support the postgresql engine; query files left unchanged diff --git a/internal/endtoend/testdata/fmt/postgresql/stdout.txt b/internal/endtoend/testdata/fmt/postgresql/stdout.txt index e69de29bb2..0f801d777b 100644 --- a/internal/endtoend/testdata/fmt/postgresql/stdout.txt +++ b/internal/endtoend/testdata/fmt/postgresql/stdout.txt @@ -0,0 +1,48 @@ +--- a/query.sql ++++ b/query.sql +@@ -1,7 +1,8 @@ + -- name: GetAuthor :one +-select id,name , bio +-from authors +-where id = $1 limit 1; -- the primary lookup ++SELECT id, name, bio ++FROM authors ++WHERE id = $1 ++LIMIT 1; -- the primary lookup + + /* This listing powers the admin page. + Keep it ordered by name so the UI stays stable. */ +@@ -8,5 +9,6 @@ + -- name: ListAuthors :many +-SELECT id, name, bio FROM authors ++SELECT id, name, bio ++FROM authors + ORDER BY name; + + -- name: CreateAuthor :one +@@ -13,8 +15,5 @@ +-INSERT INTO authors ( +- name, bio +-) VALUES ( +- $1, $2 +-) ++INSERT INTO authors (name, bio) ++VALUES ($1, $2) + RETURNING *; + + -- name: PickyQuery :many +@@ -21,5 +20,6 @@ +-SELECT id, -- the primary key +- name ++SELECT ++ id, -- the primary key ++ name + FROM authors + WHERE id > $1; + +@@ -27,4 +27,4 @@ + SELECT id, name, bio, created_at FROM authors WHERE name LIKE $1 AND bio IS NOT NULL AND id > $2 AND name <> $3 ORDER BY name, id LIMIT $4; + + -- name: DeleteAuthor :exec +-DELETE FROM authors WHERE id = @id ++DELETE FROM authors WHERE id = @id; diff --git a/internal/engine/postgresql/parse.go b/internal/engine/postgresql/parse.go index bb4c0d2a0a..0de54e6eb3 100644 --- a/internal/engine/postgresql/parse.go +++ b/internal/engine/postgresql/parse.go @@ -150,15 +150,28 @@ type Parser struct { var errSkip = errors.New("skip stmt") func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) { - contents, err := io.ReadAll(r) + f, err := p.ParseFile(r) if err != nil { return nil, err } - tree, err := Parse(string(contents)) + return f.Stmts, nil +} + +// ParseFile parses like Parse and also carries the file's comments, which +// oliphant's parser collects from the same pass its scanner already makes +// over the input. +func (p *Parser) ParseFile(r io.Reader) (*ast.File, error) { + blob, err := io.ReadAll(r) + if err != nil { + return nil, err + } + contents := string(blob) + parsed, err := parser.ParseFile(contents) if err != nil { pErr := normalizeErr(err) return nil, pErr } + tree := parsed.ParseResult var stmts []ast.Statement // PostgreSQL 18 changed stmt_location to point at the statement's first @@ -203,7 +216,33 @@ func (p *Parser) Parse(r io.Reader) ([]ast.Statement, error) { }, }) } - return stmts, nil + + var comments []ast.Comment + for _, tok := range parsed.Comments { + comments = append(comments, ast.Comment{ + Text: strings.TrimRight(contents[tok.Start:tok.End], " \t\r\n"), + Start: int(tok.Start), + End: int(tok.End), + OwnLine: ownLine(contents, int(tok.Start)), + }) + } + return &ast.File{Stmts: stmts, Comments: comments}, nil +} + +// ownLine reports that only blank space sits between the preceding line +// break and pos. +func ownLine(src string, pos int) bool { + for j := pos - 1; j >= 0; j-- { + switch src[j] { + case '\n': + return true + case ' ', '\t', '\r': + continue + default: + return false + } + } + return true } func normalizeErr(err error) error { diff --git a/internal/engine/postgresql/reserved.go b/internal/engine/postgresql/reserved.go index 25cebb16e9..8b826f3317 100644 --- a/internal/engine/postgresql/reserved.go +++ b/internal/engine/postgresql/reserved.go @@ -70,6 +70,13 @@ func (p *Parser) Cast(arg, typeName string) string { return arg + "::" + typeName } +// Fingerprint reduces a query to pg_query's fingerprint, which survives +// changes in whitespace, case and layout. sqlc fmt uses it to prove a +// formatted statement still parses to the same query. +func (p *Parser) Fingerprint(sql string) (string, error) { + return Fingerprint(sql) +} + // https://www.postgresql.org/docs/current/sql-keywords-appendix.html func (p *Parser) IsReservedKeyword(s string) bool { switch strings.ToLower(s) {