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) {