diff --git a/cmd/gosqlx/cmd/doc.go b/cmd/gosqlx/cmd/doc.go index 19ef5cc6..d3e18257 100644 --- a/cmd/gosqlx/cmd/doc.go +++ b/cmd/gosqlx/cmd/doc.go @@ -285,11 +285,18 @@ // // ## format Command // -// Formats SQL with intelligent indentation. +// Formats SQL with intelligent indentation. Rendering delegates to +// pkg/formatter, and the output is checked by re-parsing it: it must re-parse +// to the same number of statements with the same AST.SQL() and per-statement +// rendering, and keep every string literal and the meaning of every quoted +// identifier; otherwise the file is reported as an error and left unchanged. +// The check compares the input's parse with the output's parse, so anything +// the parser itself does not keep (comments, clauses it skips) is not +// detected and is dropped from the output. // // Implementation: formatter.go, sql_formatter.go // Key types: Formatter, CLIFormatterOptions, SQLFormatter -// Key functions: Format(), formatFile(), formatSQL() +// Key functions: Format(), formatFile(), formatSQL(), verifyFormattedSQL() // // ## parse Command // diff --git a/cmd/gosqlx/cmd/format_check_summary_test.go b/cmd/gosqlx/cmd/format_check_summary_test.go new file mode 100644 index 00000000..20335d36 --- /dev/null +++ b/cmd/gosqlx/cmd/format_check_summary_test.go @@ -0,0 +1,62 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestFormat_CheckSummaryOmitsFailedFiles checks that --check does not claim +// every file is properly formatted when some files failed the check. +func TestFormat_CheckSummaryOmitsFailedFiles(t *testing.T) { + dir := t.TempDir() + good := filepath.Join(dir, "good.sql") + bad := filepath.Join(dir, "bad.sql") + // EXPLAIN is refused by the formatter, so bad.sql always fails. + if err := os.WriteFile(good, []byte("SELECT a FROM t"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(bad, []byte("EXPLAIN VERBOSE DELETE FROM users"), 0600); err != nil { + t.Fatal(err) + } + + var out, errBuf bytes.Buffer + f := NewFormatter(&out, &errBuf, CLIFormatterOptions{Check: true, Compact: true, Uppercase: true}) + res, err := f.Format([]string{good, bad}) + if err != nil { + t.Fatalf("Format: %v", err) + } + if res.FailedFiles != 1 { + t.Fatalf("FailedFiles = %d, want 1 (stderr: %s)", res.FailedFiles, errBuf.String()) + } + if strings.Contains(out.String(), "properly formatted") { + t.Fatalf("check summary claims files are properly formatted although one failed:\n%s", out.String()) + } + + // With no failures the summary is still printed. + out.Reset() + errBuf.Reset() + f = NewFormatter(&out, &errBuf, CLIFormatterOptions{Check: true, Compact: true, Uppercase: true}) + if _, err := f.Format([]string{good}); err != nil { + t.Fatalf("Format: %v (stderr: %s)", err, errBuf.String()) + } + if !strings.Contains(out.String(), "All 1 files are properly formatted") { + t.Fatalf("missing check summary, got stdout %q", out.String()) + } +} diff --git a/cmd/gosqlx/cmd/format_corpus_test.go b/cmd/gosqlx/cmd/format_corpus_test.go new file mode 100644 index 00000000..cb8afde4 --- /dev/null +++ b/cmd/gosqlx/cmd/format_corpus_test.go @@ -0,0 +1,180 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" +) + +// corpusFormatKnownGaps lists corpus files (relative to testdata) whose +// formatted output does not yet round-trip because of a pkg/formatter or SQL() +// rendering gap. For these the CLI must refuse to format (return an error) +// rather than emit changed SQL; once the gap is fixed they simply pass. +var corpusFormatKnownGaps = map[string]string{ + "corpus/orm/gorm_joins.sql": "follow-up: a name quoted only for case (\"Company\") renders bare; the AST does not record quoting, so the CLI refuses", + "mssql/13_cross_apply.sql": "follow-up: CROSS APPLY rendered as CROSS APPLY JOIN, TOP lost in derived table", + "mssql/14_outer_apply.sql": "follow-up: OUTER APPLY rendered as OUTER APPLY JOIN, TOP lost in derived table", + "mysql/29_full_text_search.sql": "follow-up: MATCH ... AGAINST rendered as AGAINST AGAINST(...)", + "oracle/08_connect_by_path.sql": "follow-up: START WITH / CONNECT BY not rendered", + "oracle/19_regexp_functions.sql": "follow-up: string literal backslashes doubled for a dialect that keeps them literal", +} + +// corpusDialect maps a testdata path to the dialect its SQL is written in. +func corpusDialect(rel string) string { + p := "/" + rel + switch { + case strings.Contains(p, "/mssql/"): + return "sqlserver" + case strings.Contains(p, "/mysql/"): + return "mysql" + case strings.Contains(p, "/oracle/"): + return "oracle" + case strings.Contains(p, "/postgresql/"): + return "postgresql" + default: + return "" + } +} + +// TestFormatSQL_CorpusRoundTrip formats every parseable testdata/**/*.sql file +// with the CLI formatter in several modes and requires that the output +// re-parses to an equivalent tree and formats idempotently (#599). +func TestFormatSQL_CorpusRoundTrip(t *testing.T) { + root := filepath.Join("..", "..", "..", "testdata") + var files []string + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && strings.HasSuffix(path, ".sql") { + files = append(files, path) + } + return nil + }) + if err != nil { + t.Fatalf("walking %s: %v", root, err) + } + sort.Strings(files) + + modes := []struct { + name string + opts CLIFormatterOptions + }{ + {"readable-upper", CLIFormatterOptions{IndentSize: 2, Uppercase: true}}, + {"compact-upper", CLIFormatterOptions{Compact: true, Uppercase: true}}, + {"readable-lower", CLIFormatterOptions{IndentSize: 4, Uppercase: false}}, + } + + parsed := 0 + for _, path := range files { + relPath, err := filepath.Rel(root, path) + if err != nil { + t.Fatalf("relative path for %s: %v", path, err) + } + rel := filepath.ToSlash(relPath) + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading %s: %v", rel, err) + } + input := strings.ReplaceAll(string(data), "\r\n", "\n") + dialect := corpusDialect(rel) + + orig, err := parseWithDialect(t, input, dialect) + if err != nil || len(orig.Statements) == 0 { + if orig != nil { + ast.ReleaseAST(orig) + } + continue // the corpus also holds inputs the parser does not support yet + } + parsed++ + origSQL := orig.SQL() + origCount := len(orig.Statements) + ast.ReleaseAST(orig) + + for _, m := range modes { + opts := m.opts + opts.Dialect = dialect + out, err := formatWith(input, opts) + if err != nil { + if reason, ok := corpusFormatKnownGaps[rel]; ok { + t.Logf("%s [%s]: known gap (%s): %v", rel, m.name, reason, err) + continue + } + t.Errorf("%s [%s]: format failed: %v", rel, m.name, err) + continue + } + if reason, ok := corpusFormatKnownGaps[rel]; ok { + t.Errorf("%s [%s]: listed as a known gap (%s) but now formats; remove it from corpusFormatKnownGaps", rel, m.name, reason) + } + // The parser does not yet accept every lowercase keyword sequence + // (e.g. "returning", "cross join lateral"), so lowercase output is + // checked through its uppercase-keyword twin, mirroring the CLI's + // own verification: the two must differ only in case. + checkText, checkOpts := out, opts + if !opts.Uppercase { + checkOpts.Uppercase = true + twin, err := formatWith(input, checkOpts) + if err != nil { + t.Errorf("%s [%s]: uppercase twin failed: %v", rel, m.name, err) + continue + } + if !strings.EqualFold(out, twin) { + t.Errorf("%s [%s]: output differs from its uppercase twin beyond keyword case\nlower:\n%s\nupper:\n%s", rel, m.name, out, twin) + continue + } + checkText = twin + } + checkCorpusOutput(t, rel, m.name, checkText, dialect, origSQL, origCount) + again, err := formatWith(checkText, checkOpts) + if err != nil { + t.Errorf("%s [%s]: re-formatting output failed: %v", rel, m.name, err) + continue + } + if again != checkText { + t.Errorf("%s [%s]: formatting is not idempotent\nfirst:\n%s\nsecond:\n%s", rel, m.name, checkText, again) + } + } + } + if parsed < 100 { + t.Fatalf("only %d corpus files parsed; expected at least 100 (is testdata missing?)", parsed) + } + t.Logf("formatted %d parseable corpus files in %d modes", parsed, len(modes)) +} + +// checkCorpusOutput independently re-parses out and compares it with the +// original input's statement count and SQL(). +func checkCorpusOutput(t *testing.T, rel, mode, out, dialect, origSQL string, origCount int) { + t.Helper() + reparsed, err := parseWithDialect(t, out, dialect) + if err != nil { + t.Errorf("%s [%s]: output does not re-parse: %v\noutput:\n%s", rel, mode, err, out) + return + } + defer ast.ReleaseAST(reparsed) + if len(reparsed.Statements) != origCount { + t.Errorf("%s [%s]: statement count changed: %d -> %d", rel, mode, origCount, len(reparsed.Statements)) + return + } + if got := reparsed.SQL(); !strings.EqualFold(got, origSQL) { + t.Errorf("%s [%s]: output changes the query\n original SQL(): %s\nreparsed SQL(): %s", rel, mode, origSQL, got) + } +} diff --git a/cmd/gosqlx/cmd/format_explain_test.go b/cmd/gosqlx/cmd/format_explain_test.go new file mode 100644 index 00000000..e0e6dfc7 --- /dev/null +++ b/cmd/gosqlx/cmd/format_explain_test.go @@ -0,0 +1,131 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestFormatSQL_RefusesExplainAndDescribe guards against gosqlx format turning +// a non-executing EXPLAIN into an executable statement. The parser splits +// "EXPLAIN VERBOSE|ANALYZE " into a DescribeStatement plus , and +// DescribeStatement drops EXPLAIN options and object kinds, so the CLI must +// refuse it instead of writing "DESCRIBE VERBOSE; DELETE FROM users". +func TestFormatSQL_RefusesExplainAndDescribe(t *testing.T) { + tests := []struct { + name string + dialect string + in string + }{ + {name: "explain verbose delete", in: "EXPLAIN VERBOSE DELETE FROM users"}, + {name: "explain verbose delete postgresql", dialect: "postgresql", in: "EXPLAIN VERBOSE DELETE FROM users;\n"}, + {name: "explain verbose update", in: "EXPLAIN VERBOSE UPDATE accounts SET balance = 0"}, + {name: "explain verbose drop", in: "EXPLAIN VERBOSE DROP TABLE users"}, + {name: "explain verbose truncate", in: "EXPLAIN VERBOSE TRUNCATE TABLE users"}, + {name: "explain verbose insert", in: "EXPLAIN VERBOSE INSERT INTO t (a) VALUES (1)"}, + {name: "explain analyze delete", in: "EXPLAIN ANALYZE DELETE FROM users"}, + {name: "mysql explain analyze select", dialect: "mysql", in: "EXPLAIN ANALYZE SELECT * FROM users"}, + {name: "explain select", in: "EXPLAIN SELECT 1"}, + {name: "describe table", in: "DESCRIBE users"}, + {name: "snowflake describe table kind", dialect: "snowflake", in: "DESCRIBE TABLE users"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + for _, opts := range []CLIFormatterOptions{ + {Compact: true, Uppercase: true, Dialect: tc.dialect}, + {IndentSize: 2, Uppercase: true, Dialect: tc.dialect}, + {IndentSize: 2, Dialect: tc.dialect}, + } { + out, err := formatWith(tc.in, opts) + if err == nil { + t.Fatalf("formatSQL(%q, %+v) = %q, want an error", tc.in, opts, out) + } + if out != "" { + t.Fatalf("formatSQL(%q) returned output %q alongside error %v", tc.in, out, err) + } + } + }) + } +} + +// TestFormat_InPlaceLeavesExplainUnchanged checks the end-to-end format -i +// path: the file must be left untouched and counted as failed. +func TestFormat_InPlaceLeavesExplainUnchanged(t *testing.T) { + const original = "EXPLAIN VERBOSE DELETE FROM users;\n" + path := filepath.Join(t.TempDir(), "r.sql") + if err := os.WriteFile(path, []byte(original), 0600); err != nil { + t.Fatal(err) + } + var out, errBuf bytes.Buffer + f := NewFormatter(&out, &errBuf, CLIFormatterOptions{InPlace: true, IndentSize: 2, Uppercase: true, Dialect: "postgresql"}) + res, err := f.Format([]string{path}) + if err != nil { + t.Fatalf("Format: %v", err) + } + if res.FailedFiles != 1 || res.FormattedFiles != 0 { + t.Fatalf("FailedFiles=%d FormattedFiles=%d, want 1 and 0 (stderr: %s)", res.FailedFiles, res.FormattedFiles, errBuf.String()) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != original { + t.Fatalf("format -i rewrote the file:\n%s", got) + } +} + +// TestStatementSegments covers the separator guard that refuses parses with +// more statements than semicolon-separated segments in the input. +func TestStatementSegments(t *testing.T) { + tests := []struct { + in string + want int + }{ + {"SELECT 1", 1}, + {"SELECT 1;", 1}, + {";;SELECT 1;;", 1}, + {"SELECT 1; SELECT 2", 2}, + {"SELECT 1;\n\nSELECT 2;\n", 2}, + {"EXPLAIN VERBOSE DELETE FROM users;", 1}, + } + for _, tc := range tests { + tkz, release, err := tokenizerForDialect("") + if err != nil { + t.Fatal(err) + } + tokens, err := tkz.TokenizeString(tc.in) + if err != nil { + release() + t.Fatalf("tokenize %q: %v", tc.in, err) + } + got := statementSegments(tokens) + release() + if got != tc.want { + t.Errorf("statementSegments(%q) = %d, want %d", tc.in, got, tc.want) + } + } +} + +// TestFormatSQL_UnsupportedStatementMessage checks the refusal names the type. +func TestFormatSQL_UnsupportedStatementMessage(t *testing.T) { + _, err := formatWith("DESCRIBE users", CLIFormatterOptions{Compact: true}) + if err == nil || !strings.Contains(err.Error(), "DescribeStatement") { + t.Fatalf("want an error naming DescribeStatement, got %v", err) + } +} diff --git a/cmd/gosqlx/cmd/format_guard_test.go b/cmd/gosqlx/cmd/format_guard_test.go new file mode 100644 index 00000000..aac8cb43 --- /dev/null +++ b/cmd/gosqlx/cmd/format_guard_test.go @@ -0,0 +1,200 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/models" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" +) + +// TestFormatSQL_RefusesLostQuotes checks that the CLI refuses input whose +// quoted identifiers the renderer would write bare when that changes the +// identifier (reserved word, case folding, special characters). AST.SQL() +// cannot see this: identifiers carry no quoting, and statements such as ALTER +// have no SQL() at all, so the old check compared "" with "". +func TestFormatSQL_RefusesLostQuotes(t *testing.T) { + tests := []struct { + name string + dialect string + in string + }{ + {name: "alter mixed case", in: `ALTER TABLE t ADD COLUMN "Mixed" INT`}, + // Refused by checkMySQLDoubleQuotes: in ALTER, `select` would render as + // "select". (In a SELECT list checkQuotedTokens refuses it first.) + {name: "alter mysql backtick keyword", dialect: "mysql", in: "ALTER TABLE `Users` ADD COLUMN `select` INT"}, + {name: "select mixed case", in: `SELECT "Mixed" FROM t`}, + {name: "select mixed case table", in: `SELECT a FROM "Mixed"`}, + {name: "create table quoted", in: `CREATE TABLE "Mixed" ("order" INT)`}, + {name: "oracle lowercase quoted", dialect: "oracle", in: `SELECT "abc" FROM t`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + for _, opts := range []CLIFormatterOptions{ + {Compact: true, Uppercase: true, Dialect: tc.dialect}, + {IndentSize: 2, Dialect: tc.dialect}, + } { + out, err := formatWith(tc.in, opts) + if err == nil { + t.Fatalf("formatSQL(%q, %+v) = %q, want an error", tc.in, opts, out) + } + if out != "" { + t.Fatalf("formatSQL(%q) returned output %q alongside error %v", tc.in, out, err) + } + } + }) + } +} + +// TestFormatSQL_AllowsHarmlessQuotes checks that quoted identifiers that mean +// the same thing bare are still formatted. +func TestFormatSQL_AllowsHarmlessQuotes(t *testing.T) { + tests := []struct { + name string + dialect string + in string + want string + }{ + {name: "lowercase quoted", in: `SELECT "users_id" FROM "users"`, want: "SELECT users_id FROM users"}, + {name: "alter plain", in: "ALTER TABLE t ADD COLUMN c INT", want: "ALTER TABLE t ADD COLUMN c INT"}, + // Reserved words keep their quotes on render (#632), so these format. + {name: "alter reserved word", in: `ALTER TABLE t ADD COLUMN "order" INT`, want: `ALTER TABLE t ADD COLUMN "order" INT`}, + {name: "alter reserved word postgresql", dialect: "postgresql", in: `ALTER TABLE t ADD COLUMN "order" INT`, want: `ALTER TABLE t ADD COLUMN "order" INT`}, + {name: "mysql backtick plain", dialect: "mysql", in: "ALTER TABLE `Users` ADD COLUMN `c` INT", want: "ALTER TABLE Users ADD COLUMN c INT"}, + {name: "sqlserver bracket kept quoted", dialect: "sqlserver", in: "SELECT [Mixed Name] FROM t", want: `SELECT "Mixed Name" FROM t`}, + {name: "oracle uppercase quoted", dialect: "oracle", in: `SELECT "ABC" FROM t`, want: "SELECT ABC FROM t"}, + {name: "string literal kept", in: "SELECT a FROM t WHERE b = 'it''s'", want: "SELECT a FROM t WHERE b = 'it''s'"}, + {name: "mysql escaped literal", dialect: "mysql", in: `SELECT 'a\nb'`, want: `SELECT 'a\nb'`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + out, err := formatWith(tc.in, CLIFormatterOptions{Compact: true, Uppercase: true, Dialect: tc.dialect}) + if err != nil { + t.Fatalf("formatSQL(%q): %v", tc.in, err) + } + if out != tc.want { + t.Fatalf("formatSQL(%q) = %q, want %q", tc.in, out, tc.want) + } + }) + } +} + +// TestFormatSQL_RefusesBackslashLiteralChange checks that string literals +// with backslashes are never rewritten in dialects where backslash is not an +// escape character (the renderer doubles them); the CLI must refuse. +func TestFormatSQL_RefusesBackslashLiteralChange(t *testing.T) { + for _, d := range []string{"sqlserver", "oracle", "sqlite", "snowflake", "clickhouse"} { + t.Run(d, func(t *testing.T) { + in := `SELECT 'C:\temp\x'` + out, err := formatWith(in, CLIFormatterOptions{Compact: true, Uppercase: true, Dialect: d}) + if err == nil { + t.Fatalf("formatSQL(%q, %s) = %q, want an error", in, d, out) + } + if !strings.Contains(err.Error(), "string literal") { + t.Fatalf("error %q does not name the changed string literal", err) + } + }) + } +} + +// TestVerifyFormattedSQL_StatementsWithoutSQL checks that the verification +// does not trivially pass for statements that have no SQL() method (e.g. +// ALTER), whose AST.SQL() contribution is empty on both sides. +func TestVerifyFormattedSQL_StatementsWithoutSQL(t *testing.T) { + const in = "ALTER TABLE t ADD COLUMN c INT" + tkz, release, err := tokenizerForDialect("") + if err != nil { + t.Fatal(err) + } + defer release() + tokens, err := tkz.TokenizeString(in) + if err != nil { + t.Fatal(err) + } + orig, err := parserForDialect("").ParseFromModelTokens(tokens) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(orig) + if err := verifyFormattedSQL(orig, tokens, "ALTER TABLE t ADD COLUMN c INT", ""); err != nil { + t.Fatalf("identical output rejected: %v", err) + } + if err := verifyFormattedSQL(orig, tokens, "ALTER TABLE t ADD COLUMN d BIGINT", ""); err == nil { + t.Fatal("output with a different ALTER statement was accepted") + } +} + +// TestFormatSQL_RefusesDescribeObjectKinds checks that DESCRIBE statements +// whose object kind the AST does not keep are refused rather than rewritten +// to a different command (e.g. Snowflake DESCRIBE STAGE -> DESCRIBE). +func TestFormatSQL_RefusesDescribeObjectKinds(t *testing.T) { + for _, in := range []string{"DESCRIBE STAGE my_stage", "DESCRIBE VIEW user_summary", "DESC TABLE users"} { + t.Run(in, func(t *testing.T) { + out, err := formatWith(in, CLIFormatterOptions{Compact: true, Uppercase: true, Dialect: "snowflake"}) + if err == nil { + t.Fatalf("formatSQL(%q) = %q, want an error", in, out) + } + }) + } +} + +// TestCheckMySQLDoubleQuotes checks that MySQL/MariaDB output keeps exactly +// the input's double-quoted tokens, and that other dialects are not checked. +func TestCheckMySQLDoubleQuotes(t *testing.T) { + tok := func(v string, q rune) models.TokenWithSpan { + return models.TokenWithSpan{Token: models.Token{Type: models.TokenTypeIdentifier, Value: v, Quote: q}} + } + type toks = []models.TokenWithSpan + tests := []struct { + name string + in, out toks + wantErr bool + }{ + {"kept", toks{tok("bob", '"')}, toks{tok("bob", '"')}, false}, + {"string written bare", toks{tok("bob", '"')}, toks{tok("bob", 0)}, true}, + {"identifier newly double-quoted", toks{tok("select", '`')}, toks{tok("select", '"')}, true}, + {"count bypass", toks{tok("abc", '"'), tok("my col", '`')}, toks{tok("abc", 0), tok("my col", '"')}, true}, + {"no double quotes", toks{tok("a", '`')}, toks{tok("a", 0)}, false}, + {"ANSI_QUOTES qualifier", toks{tok("users", '"'), tok(".", 0), tok("id", '"')}, toks{tok("users", 0), tok(".", 0), tok("id", 0)}, false}, + {"ANSI_QUOTES table", toks{tok("FROM", 0), tok("users", '"')}, toks{tok("FROM", 0), tok("users", 0)}, false}, + } + for _, tc := range tests { + for _, d := range []string{"mysql", "MariaDB"} { + if err := checkMySQLDoubleQuotes(tc.in, tc.out, d); (err != nil) != tc.wantErr { + t.Errorf("%s/%s: err = %v, wantErr %v", tc.name, d, err, tc.wantErr) + } + } + for _, d := range []string{"", "postgresql", "sqlserver"} { + if err := checkMySQLDoubleQuotes(tc.in, tc.out, d); err != nil { + t.Errorf("%s/%q: %v, want no check outside MySQL/MariaDB", tc.name, d, err) + } + } + } +} + +// TestFormatSQL_MySQLDoubleQuotedString checks the end-to-end refusal of a +// MySQL double-quoted string that the renderer would write bare. +func TestFormatSQL_MySQLDoubleQuotedString(t *testing.T) { + for _, in := range []string{ + `SELECT a FROM t WHERE name = "bob"`, + "SELECT \"abc\", `my col` FROM t", + } { + if out, err := formatWith(in, CLIFormatterOptions{Compact: true, Dialect: "mysql"}); err == nil { + t.Errorf("formatSQL(%q, mysql) = %q, want an error", in, out) + } + } +} diff --git a/cmd/gosqlx/cmd/format_issue599_test.go b/cmd/gosqlx/cmd/format_issue599_test.go new file mode 100644 index 00000000..697dbcf7 --- /dev/null +++ b/cmd/gosqlx/cmd/format_issue599_test.go @@ -0,0 +1,195 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "bytes" + "strings" + "testing" + + pkgformatter "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" +) + +// parseWithDialect tokenizes and parses sql with the CLI's dialect-aware +// tokenizer and parser. The caller must release the returned AST. +func parseWithDialect(t *testing.T, sql, dialect string) (*ast.AST, error) { + t.Helper() + tkz, release, err := tokenizerForDialect(dialect) + if err != nil { + return nil, err + } + defer release() + tokens, err := tkz.TokenizeString(sql) + if err != nil { + return nil, err + } + return parserForDialect(dialect).ParseFromModelTokens(tokens) +} + +// formatWith runs the CLI formatSQL path with the given options. +func formatWith(sql string, opts CLIFormatterOptions) (string, error) { + var out, errBuf bytes.Buffer + return NewFormatter(&out, &errBuf, opts).formatSQL(sql) +} + +// assertRoundTrip checks that formatted re-parses with dialect to a tree whose +// SQL() matches the original input's SQL() (ignoring case), with the same +// statement count. +func assertRoundTrip(t *testing.T, input, formatted, dialect string) { + t.Helper() + orig, err := parseWithDialect(t, input, dialect) + if err != nil { + t.Fatalf("input does not parse: %v", err) + } + defer ast.ReleaseAST(orig) + reparsed, err := parseWithDialect(t, formatted, dialect) + if err != nil { + t.Fatalf("formatted output does not re-parse: %v\noutput:\n%s", err, formatted) + } + defer ast.ReleaseAST(reparsed) + if len(orig.Statements) != len(reparsed.Statements) { + t.Fatalf("statement count changed: %d -> %d\noutput:\n%s", len(orig.Statements), len(reparsed.Statements), formatted) + } + if o, r := orig.SQL(), reparsed.SQL(); !strings.EqualFold(o, r) { + t.Fatalf("formatted output changes the query\n original SQL(): %s\nreparsed SQL(): %s\noutput:\n%s", o, r, formatted) + } +} + +// libraryCompact renders input with pkg/formatter using the same style the CLI +// adapter builds for compact, uppercase-keyword output. +func libraryCompact(t *testing.T, input, dialect string) string { + t.Helper() + tree, err := parseWithDialect(t, input, dialect) + if err != nil { + t.Fatalf("input does not parse: %v", err) + } + defer ast.ReleaseAST(tree) + style := ast.CompactStyle() + style.KeywordCase = ast.KeywordUpper + style.AddSemicolon = false + style.Dialect = dialect + parts := make([]string, 0, len(tree.Statements)) + for _, stmt := range tree.Statements { + parts = append(parts, pkgformatter.FormatStatement(stmt, style)) + } + return strings.Join(parts, "; ") +} + +// TestFormatSQL_Issue599 reproduces every case from issue #599: the CLI +// formatter used its own renderer, which replaced unhandled expressions with +// their token literal and dropped INSERT clauses, exiting 0 with SQL of a +// different meaning. Rows with blockedOn set are lost by pkg/formatter itself; +// they skip (after checking the CLI either refuses the output or matches the +// library) until the referenced library fix lands, and never fail once it does. +func TestFormatSQL_Issue599(t *testing.T) { + tests := []struct { + name string + dialect string + in string + want string // exact compact output (or substring when contains) + contains bool + blockedOn string + }{ + {name: "cast and double colon", in: "SELECT CAST(a AS INT), b::text FROM t", want: "SELECT CAST(a AS INT), CAST(b AS text) FROM t"}, + {name: "extract", in: "SELECT EXTRACT(YEAR FROM d) FROM t", want: "SELECT EXTRACT(YEAR FROM d) FROM t"}, + {name: "interval", in: "SELECT d + INTERVAL '1 day' FROM t", want: "SELECT d + INTERVAL '1 day' FROM t"}, + {name: "interval frame bound", in: "SELECT SUM(x) OVER (ORDER BY d RANGE BETWEEN INTERVAL '1 day' PRECEDING AND CURRENT ROW) FROM t", want: "SELECT SUM(x) OVER (ORDER BY d RANGE BETWEEN INTERVAL '1 day' PRECEDING AND CURRENT ROW) FROM t"}, + {name: "any all", in: "SELECT * FROM t WHERE a = ANY(SELECT x FROM u) AND b > ALL(SELECT x FROM u)", want: "SELECT * FROM t WHERE a = ANY (SELECT x FROM u) AND b > ALL (SELECT x FROM u)"}, + {name: "tuples", in: "SELECT * FROM t WHERE (a, b) IN ((1, 2), (3, 4))", want: "SELECT * FROM t WHERE (a, b) IN ((1, 2), (3, 4))"}, + {name: "subscripts slices array", in: "SELECT arr[1], arr[1:2], ARRAY[1,2] FROM t", want: "SELECT arr[1], arr[1:2], ARRAY[1, 2] FROM t"}, + {name: "rollup", in: "SELECT a, b, COUNT(*) FROM t GROUP BY ROLLUP(a, b)", want: "SELECT a, b, COUNT(*) FROM t GROUP BY ROLLUP(a, b)"}, + {name: "cube", in: "SELECT a, b, COUNT(*) FROM t GROUP BY CUBE(a, b)", want: "SELECT a, b, COUNT(*) FROM t GROUP BY CUBE(a, b)"}, + {name: "grouping sets", in: "SELECT a, b, COUNT(*) FROM t GROUP BY GROUPING SETS ((a), (b), ())", want: "SELECT a, b, COUNT(*) FROM t GROUP BY GROUPING SETS((a), (b), ())"}, + {name: "within group", in: "SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x) FROM t", want: "SELECT PERCENTILE_CONT(0.5) WITHIN GROUP (ORDER BY x) FROM t"}, + {name: "exists set operation", in: "SELECT * FROM t WHERE EXISTS (SELECT 1 UNION SELECT 2)", want: "SELECT * FROM t WHERE EXISTS (SELECT 1 UNION SELECT 2)"}, + {name: "in set operation", in: "SELECT * FROM t WHERE a IN (SELECT 1 UNION SELECT 2)", want: "SELECT * FROM t WHERE a IN (SELECT 1 UNION SELECT 2)"}, + {name: "insert on conflict", in: "INSERT INTO t (a) VALUES (1) ON CONFLICT (a) DO UPDATE SET a = 2", want: "INSERT INTO t (a) VALUES (1) ON CONFLICT (a) DO UPDATE SET a = 2"}, + {name: "insert returning", in: "INSERT INTO t (a) VALUES (1) RETURNING id", want: "INSERT INTO t (a) VALUES (1) RETURNING id"}, + {name: "sqlserver pivot", dialect: "sqlserver", in: "SELECT * FROM s PIVOT (SUM(amt) FOR q IN ([Q1], [Q2])) AS p", want: "SELECT * FROM s PIVOT (SUM(amt) FOR q IN ([Q1], [Q2])) AS p"}, + + // Already correct before #599; guard against regressions. + {name: "rows current row", in: "SELECT SUM(x) OVER (ORDER BY d ROWS CURRENT ROW) FROM t", want: "SELECT SUM(x) OVER (ORDER BY d ROWS CURRENT ROW) FROM t"}, + {name: "rows numeric frame", in: "SELECT SUM(x) OVER (ORDER BY d ROWS BETWEEN 2 PRECEDING AND CURRENT ROW) FROM t", want: "SELECT SUM(x) OVER (ORDER BY d ROWS BETWEEN 2 PRECEDING AND CURRENT ROW) FROM t"}, + {name: "lowercase set operation", in: "select a from t union select b from u", want: "SELECT a FROM t UNION SELECT b FROM u"}, + {name: "statement separator", in: "SELECT 1; SELECT 2", want: "SELECT 1; SELECT 2"}, + + // ALTER TABLE: the old CLI emitted `ADD c INT`, a bare `DROP`, + // `ADD CONSTRAINT pk`, `RENAME COLUMN TO b` and `...`. + {name: "alter add column", in: "ALTER TABLE t ADD COLUMN c INT NOT NULL DEFAULT 0", want: "ALTER TABLE t ADD COLUMN c INT NOT NULL DEFAULT 0"}, + {name: "alter drop column cascade", in: "ALTER TABLE t DROP COLUMN c CASCADE", want: "ALTER TABLE t DROP COLUMN c CASCADE"}, + {name: "alter add primary key", in: "ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (id)", want: "ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (id)"}, + {name: "alter add foreign key", in: "ALTER TABLE t ADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES u (id) ON DELETE CASCADE", want: "ALTER TABLE t ADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES u (id) ON DELETE CASCADE"}, + {name: "alter drop constraint", in: "ALTER TABLE t DROP CONSTRAINT pk", want: "ALTER TABLE t DROP CONSTRAINT pk"}, + {name: "alter rename column", in: "ALTER TABLE t RENAME COLUMN a TO b", want: "ALTER TABLE t RENAME COLUMN a TO b"}, + {name: "alter rename table", in: "ALTER TABLE t RENAME TO u", want: "ALTER TABLE t RENAME TO u"}, + + // Lost by pkg/formatter itself; fixed by the referenced library issue. + {name: "named argument", in: "SELECT f(x => 1) FROM t", want: "SELECT f(x => 1) FROM t", blockedOn: "#600"}, + {name: "ignore nulls", in: "SELECT FIRST_VALUE(x) IGNORE NULLS OVER (ORDER BY y) FROM t", want: "SELECT FIRST_VALUE(x) IGNORE NULLS OVER (ORDER BY y) FROM t", blockedOn: "#600"}, + {name: "clickhouse settings", dialect: "clickhouse", in: "SELECT a FROM t SETTINGS max_threads = 4", want: "SELECT a FROM t SETTINGS max_threads = 4", blockedOn: "#600"}, + {name: "mysql on duplicate key", dialect: "mysql", in: "INSERT INTO t (a) VALUES (1) ON DUPLICATE KEY UPDATE a = 2", want: "INSERT INTO t (a) VALUES (1) ON DUPLICATE KEY UPDATE a = 2", blockedOn: "#600"}, + {name: "sqlserver insert output", dialect: "sqlserver", in: "INSERT INTO t (a) OUTPUT INSERTED.id VALUES (1)", want: "INSERT INTO t (a) OUTPUT INSERTED.id VALUES (1)", blockedOn: "#600"}, + {name: "join using", in: "SELECT * FROM a JOIN b USING (id)", want: "USING (id)", contains: true, blockedOn: "#603"}, + {name: "sqlserver nolock", dialect: "sqlserver", in: "SELECT * FROM t WITH (NOLOCK)", want: "SELECT * FROM t WITH (NOLOCK)", blockedOn: "#604"}, + {name: "not exists", in: "SELECT * FROM t WHERE NOT EXISTS (SELECT 1 FROM u)", want: "SELECT * FROM t WHERE NOT EXISTS (SELECT 1 FROM u)", blockedOn: "#601"}, + {name: "quoted alias", dialect: "postgresql", in: `SELECT a AS "my alias" FROM t`, want: `SELECT a AS "my alias" FROM t`, blockedOn: "#611"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + matches := func(out string) bool { + if tc.contains { + return strings.Contains(out, tc.want) + } + return out == tc.want + } + + out, err := formatWith(tc.in, CLIFormatterOptions{Compact: true, Uppercase: true, Dialect: tc.dialect}) + if tc.blockedOn != "" && (err != nil || !matches(out)) { + if err != nil { + if !strings.Contains(err.Error(), "formatted output") { + t.Fatalf("expected the round-trip safety net to refuse the output, got: %v", err) + } + t.Skipf("blocked on %s: %v", tc.blockedOn, err) + } + if lib := libraryCompact(t, tc.in, tc.dialect); out != lib { + t.Fatalf("CLI output diverges from pkg/formatter\n cli: %s\nlibrary: %s", out, lib) + } + t.Skipf("blocked on %s: got %q", tc.blockedOn, out) + } + if err != nil { + t.Fatalf("compact formatSQL(%q) error: %v", tc.in, err) + } + if !matches(out) { + t.Fatalf("compact formatSQL(%q)\n got: %s\nwant: %s", tc.in, out, tc.want) + } + + readableOpts := CLIFormatterOptions{IndentSize: 2, Uppercase: true, Dialect: tc.dialect} + readable, err := formatWith(tc.in, readableOpts) + if err != nil { + t.Fatalf("readable formatSQL(%q) error: %v", tc.in, err) + } + assertRoundTrip(t, tc.in, readable, tc.dialect) + again, err := formatWith(readable, readableOpts) + if err != nil { + t.Fatalf("re-formatting readable output failed: %v\noutput:\n%s", err, readable) + } + if again != readable { + t.Fatalf("readable formatting is not idempotent\nfirst:\n%s\nsecond:\n%s", readable, again) + } + }) + } +} diff --git a/cmd/gosqlx/cmd/formatter.go b/cmd/gosqlx/cmd/formatter.go index 3e39d164..9bbe77de 100644 --- a/cmd/gosqlx/cmd/formatter.go +++ b/cmd/gosqlx/cmd/formatter.go @@ -15,6 +15,7 @@ package cmd import ( + "errors" "fmt" "io" "os" @@ -22,7 +23,10 @@ import ( "time" "github.com/ajitpratap0/GoSQLX/cmd/gosqlx/internal/config" + pkgformatter "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/models" "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer" ) // CLIFormatterOptions contains configuration for the SQL formatter CLI @@ -157,7 +161,11 @@ func (f *Formatter) Format(args []string) (*FormatterResult, error) { } return result, fmt.Errorf("%d files need formatting", len(result.NeedsFormatting)) } - fmt.Fprintf(f.Out, "All %d files are properly formatted (%v)\n", result.TotalFiles, result.Duration) + // Failed files were already reported on stderr and fail the run; do + // not also claim they are properly formatted. + if result.FailedFiles == 0 { + fmt.Fprintf(f.Out, "All %d files are properly formatted (%v)\n", result.TotalFiles, result.Duration) + } } return result, nil @@ -234,6 +242,10 @@ func (f *Formatter) formatSQL(sql string) (string, error) { ast.ReleaseAST(parsedAST) }() + if err := checkFormattable(parsedAST, tokens); err != nil { + return "", err + } + // Configure formatter options indentStr := strings.Repeat(" ", f.Opts.IndentSize) formatterOpts := FormatterOptions{ @@ -241,18 +253,316 @@ func (f *Formatter) formatSQL(sql string) (string, error) { Compact: f.Opts.Compact, UppercaseKw: f.Opts.Uppercase, AlignColumns: !f.Opts.Compact, // Align columns unless in compact mode + Dialect: f.Opts.Dialect, } // Create formatter and format the AST formatter := NewSQLFormatter(formatterOpts) formatted, err := formatter.Format(parsedAST) if err != nil { - return "", fmt.Errorf("formatting failed: %w", err) + return "", fmt.Errorf("render: %w", err) + } + + // Refuse output that fails verifyFormattedSQL, so format, format -i and + // format -o do not emit or write SQL the renderer got wrong (#599). The + // check cannot see what the parser itself drops; see verifyFormattedSQL. + // Keyword case is semantically irrelevant, and the parser does not accept + // every lowercase keyword sequence, so lowercase output is verified via an + // uppercase-keyword rendering of the same tree. + verifyText := formatted + if !formatterOpts.UppercaseKw { + upperOpts := formatterOpts + upperOpts.UppercaseKw = true + verifyText, err = NewSQLFormatter(upperOpts).Format(parsedAST) + if err != nil { + return "", fmt.Errorf("render for verification: %w", err) + } + } + // verifyFormattedSQL errors are self-describing; callers add the + // "formatting failed" prefix. + if err := verifyFormattedSQL(parsedAST, tokens, verifyText, f.Opts.Dialect); err != nil { + return "", err } return formatted, nil } +// verifyFormattedSQL re-parses formatted with the same dialect and checks +// that it yields the same number of statements, the same AST.SQL(), the same +// canonical pkg/formatter rendering of each statement, and the same string +// literal values and quoted identifiers as the input tokens. +// +// SQL() is compared case-insensitively because some nodes (e.g. set operation +// operators) keep the source keyword case. The per-statement rendering covers +// statements without a SQL() method (e.g. ALTER), which AST.SQL() skips. The +// token comparison covers what neither can see: identifiers carry no quoting +// in the AST, and string literals are re-escaped by the renderer. +// +// This is a consistency check between the input's parse and the output's +// parse. Anything the parser itself drops (comments, clauses it skips) is +// absent from both and is not detected. +func verifyFormattedSQL(original *ast.AST, inputTokens []models.TokenWithSpan, formatted, dialect string) error { + tkz, release, err := tokenizerForDialect(dialect) + if err != nil { + return fmt.Errorf("tokenizer setup failed: %w", err) + } + defer release() + + tokens, err := tkz.TokenizeString(formatted) + if err != nil { + return fmt.Errorf("formatted output does not re-tokenize (formatter bug; input left unchanged): %w", err) + } + if err := checkQuotedTokens(inputTokens, tokens, dialect); err != nil { + return err + } + if err := checkMySQLDoubleQuotes(inputTokens, tokens, dialect); err != nil { + return err + } + reparsed, err := parserForDialect(dialect).ParseFromModelTokens(tokens) + if err != nil { + return fmt.Errorf("formatted output does not re-parse (formatter bug; input left unchanged): %w", err) + } + defer ast.ReleaseAST(reparsed) + + if got, want := len(reparsed.Statements), len(original.Statements); got != want { + return fmt.Errorf("formatted output has %d statements, input has %d (formatter bug; input left unchanged)", got, want) + } + if !strings.EqualFold(original.SQL(), reparsed.SQL()) { + return errors.New("formatted output changes the query (formatter bug; input left unchanged)") + } + canonical := NewSQLFormatter(FormatterOptions{Compact: true, UppercaseKw: true, Dialect: dialect}).style + for i, stmt := range original.Statements { + want := pkgformatter.FormatStatement(stmt, canonical) + got := pkgformatter.FormatStatement(reparsed.Statements[i], canonical) + if !strings.EqualFold(want, got) { + return fmt.Errorf("formatted output changes statement %d (formatter bug; input left unchanged)", i+1) + } + } + return nil +} + +// checkQuotedTokens compares the quoted tokens of the input and of the +// formatted output. Every string literal value of the input must appear in +// the output, so a literal the renderer re-escapes for another dialect (e.g. +// doubled backslashes where a backslash is not an escape) is refused. Every +// quoted identifier of the input must either appear quoted in the output or +// mean the same identifier bare (see bareWords.safe); the AST does not record +// quoting, so the renderer writes most identifiers bare. +func checkQuotedTokens(input, output []models.TokenWithSpan, dialect string) error { + bw, release, err := newBareWords(dialect) + if err != nil { + return fmt.Errorf("tokenizer setup failed: %w", err) + } + defer release() + + pool := make(map[string]int) + for _, t := range output { + if isStringLiteralToken(t.Token) || bw.isQuotedIdent(t.Token) { + pool[t.Token.Value]++ + } + } + for _, t := range input { + if !isStringLiteralToken(t.Token) { + continue + } + if pool[t.Token.Value] == 0 { + return fmt.Errorf("formatted output changes string literal %q (formatter bug; input left unchanged)", t.Token.Value) + } + pool[t.Token.Value]-- + } + for _, t := range input { + if !bw.isQuotedIdent(t.Token) { + continue + } + v := t.Token.Value + if pool[v] > 0 { + pool[v]-- + continue + } + if !bw.safe(v) { + return fmt.Errorf("formatted output drops the quotes of identifier %q (formatter bug; input left unchanged)", v) + } + } + return nil +} + +// checkMySQLDoubleQuotes requires MySQL/MariaDB output to keep exactly the +// input's double-quoted tokens. MySQL reads "..." as a string literal unless +// ANSI_QUOTES is set, while the parser treats it as an identifier, so the +// round-trip check alone misses two meaning changes: a string written bare +// (WHERE name = "bob" -> WHERE name = bob, a column reference) and an +// identifier newly written in ANSI quotes (`select` -> "select", a string). +// Input that uses a double-quoted name where a string cannot appear (a +// qualifier such as "users"."id", or a table after FROM/JOIN/INTO/UPDATE/ +// TABLE) is ANSI_QUOTES-style SQL, where "..." is an identifier, so it is +// not checked. +func checkMySQLDoubleQuotes(input, output []models.TokenWithSpan, dialect string) error { + if d := strings.ToLower(dialect); d != "mysql" && d != "mariadb" { + return nil + } + if usesANSIQuotes(input) { + return nil + } + pool := make(map[string]int) + for _, t := range output { + if t.Token.Quote == '"' { + pool[t.Token.Value]++ + } + } + for _, t := range input { + if t.Token.Quote != '"' { + continue + } + if pool[t.Token.Value] == 0 { + return fmt.Errorf("formatted output drops the double quotes of %q, which %s reads as a string (formatter limitation; input left unchanged)", t.Token.Value, dialect) + } + pool[t.Token.Value]-- + } + for v, n := range pool { + if n > 0 { + return fmt.Errorf("formatted output writes %q in ANSI double quotes, which %s reads as a string unless ANSI_QUOTES is set (formatter limitation; input left unchanged)", v, dialect) + } + } + return nil +} + +// usesANSIQuotes reports whether toks use a double-quoted token as a name in +// a position where MySQL cannot accept a string: next to a '.', or right +// after FROM, JOIN, INTO, UPDATE or TABLE. +func usesANSIQuotes(toks []models.TokenWithSpan) bool { + for i, t := range toks { + if t.Token.Quote != '"' { + continue + } + if i+1 < len(toks) && toks[i+1].Token.Value == "." { + return true + } + if i > 0 { + prev := toks[i-1].Token + if prev.Value == "." { + return true + } + switch strings.ToUpper(prev.Value) { + case "FROM", "JOIN", "INTO", "UPDATE", "TABLE": + return true + } + } + } + return false +} + +// isStringLiteralToken reports whether tok is a string literal of any form. +// Double-quoted strings are treated as identifiers (see bareWords.isQuotedIdent). +func isStringLiteralToken(tok models.Token) bool { + switch tok.Type { + case models.TokenTypeString, models.TokenTypeSingleQuotedString, + models.TokenTypeTripleSingleQuotedString, models.TokenTypeTripleDoubleQuotedString, + models.TokenTypeDollarQuotedString, models.TokenTypeByteStringLiteral, + models.TokenTypeNationalStringLiteral, models.TokenTypeEscapedStringLiteral, + models.TokenTypeUnicodeStringLiteral, models.TokenTypeHexStringLiteral: + return true + } + return tok.Quote == '\'' || tok.Quote == 'N' +} + +// bareWords classifies identifiers for one dialect, memoizing the tokenizer +// round trip that tells keywords from plain identifiers. +type bareWords struct { + dialect string + tkz *tokenizer.Tokenizer + memo map[string]bool +} + +func newBareWords(dialect string) (*bareWords, func(), error) { + tkz, release, err := tokenizerForDialect(dialect) + if err != nil { + return nil, nil, err + } + return &bareWords{dialect: dialect, tkz: tkz, memo: make(map[string]bool)}, release, nil +} + +// isQuotedIdent reports whether tok is a quoted identifier: a double-quoted +// or bracketed token, or an identifier whose value cannot be a bare word. The +// tokenizer does not record backtick quoting, so a backtick identifier is +// recognized by the latter (e.g. `select`, `a b`); one that is a plain bare +// word (`users`) is indistinguishable and harmless to unquote. +func (b *bareWords) isQuotedIdent(tok models.Token) bool { + if tok.Type == models.TokenTypeDoubleQuotedString { + return true + } + switch tok.Quote { + case '"', '`', '[': + return true + } + if tok.Word != nil && tok.Word.QuoteStyle != 0 { + return true + } + return tok.Type == models.TokenTypeIdentifier && !b.isBareWord(tok.Value) +} + +// safe reports whether the quoted identifier v names the same identifier when +// written bare: it must be a bare word (see isBareWord) and survive the +// dialect's case folding (PostgreSQL and the default dialect fold bare +// identifiers to lowercase, Oracle and Snowflake to uppercase). +func (b *bareWords) safe(v string) bool { + if !b.isBareWord(v) { + return false + } + switch b.dialect { + case "", "postgresql": + return v == strings.ToLower(v) + case "oracle", "snowflake": + return v == strings.ToUpper(v) + } + return true +} + +// isBareWord reports whether v is an ASCII word ([A-Za-z_][A-Za-z0-9_]*) that +// the dialect's tokenizer reads back as a plain identifier, i.e. not a +// keyword. +func (b *bareWords) isBareWord(v string) bool { + if ok, seen := b.memo[v]; seen { + return ok + } + ok := asciiWord(v) && b.tokenizesAsIdentifier(v) + b.memo[v] = ok + return ok +} + +func asciiWord(v string) bool { + if v == "" { + return false + } + for i := 0; i < len(v); i++ { + c := v[i] + switch { + case c == '_', c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z': + case c >= '0' && c <= '9' && i > 0: + default: + return false + } + } + return true +} + +func (b *bareWords) tokenizesAsIdentifier(v string) bool { + toks, err := b.tkz.TokenizeString(v) + if err != nil { + return false + } + n := 0 + for _, t := range toks { + if t.Token.Type == models.TokenTypeEOF { + continue + } + n++ + if n > 1 || t.Token.Type != models.TokenTypeIdentifier || t.Token.Value != v { + return false + } + } + return n == 1 +} + // FormatterFlags represents CLI flags for formatter command type FormatterFlags struct { InPlace bool @@ -307,3 +617,46 @@ func FormatterOptionsFromConfig(cfg *config.Config, flagsChanged map[string]bool return opts } + +// checkFormattable refuses parse results the CLI cannot rewrite faithfully, so +// format, format -i and format -o leave such input unchanged. +// +// DescribeStatement keeps only a name: it drops EXPLAIN options, the explained +// query and DESCRIBE object kinds, and "EXPLAIN VERBOSE|ANALYZE " parses +// as a DescribeStatement followed by , which would rewrite a +// non-executing EXPLAIN into the executable statement. UnsupportedStatement is +// rendered as a comment. Both are refused by type. As a general guard, a parse +// with more statements than semicolon-separated segments in the input means +// the parser split a statement, which re-parsing the output cannot detect. +func checkFormattable(tree *ast.AST, tokens []models.TokenWithSpan) error { + for _, stmt := range tree.Statements { + switch stmt.(type) { + case *ast.DescribeStatement, *ast.UnsupportedStatement: + return fmt.Errorf("unsupported statement type: %T (input left unchanged)", stmt) + } + } + if got, segs := len(tree.Statements), statementSegments(tokens); got > segs { + return fmt.Errorf("input parses as %d statements but has %d semicolon-separated statements (input left unchanged)", got, segs) + } + return nil +} + +// statementSegments counts the non-empty runs of tokens between semicolons, +// ignoring EOF, i.e. the most statements the input can legitimately hold. +func statementSegments(tokens []models.TokenWithSpan) int { + segs := 0 + inSeg := false + for _, tok := range tokens { + switch tok.Token.Type { + case models.TokenTypeSemicolon: + inSeg = false + case models.TokenTypeEOF: + default: + if !inSeg { + segs++ + inSeg = true + } + } + } + return segs +} diff --git a/cmd/gosqlx/cmd/insert_update_forms_format_test.go b/cmd/gosqlx/cmd/insert_update_forms_format_test.go index ef872075..bdae5cc6 100644 --- a/cmd/gosqlx/cmd/insert_update_forms_format_test.go +++ b/cmd/gosqlx/cmd/insert_update_forms_format_test.go @@ -36,7 +36,8 @@ func TestFormatCommand_InsertUpdateForms(t *testing.T) { "UPDATE t SET a = s.b FROM s WHERE t.id = s.id"}, {"update from list", "postgresql", "UPDATE t SET a = s.b FROM s, u AS v WHERE t.id = s.id", - "UPDATE t SET a = s.b FROM s, u AS v WHERE t.id = s.id"}, + // pkg/formatter writes table aliases without AS (#599). + "UPDATE t SET a = s.b FROM s, u v WHERE t.id = s.id"}, {"update from", "sqlserver", "UPDATE t SET a = s.b FROM s WHERE t.id = s.id", "UPDATE t SET a = s.b FROM s WHERE t.id = s.id"}, diff --git a/cmd/gosqlx/cmd/join_using_format_test.go b/cmd/gosqlx/cmd/join_using_format_test.go index 41fa6ce4..9f0b41c0 100644 --- a/cmd/gosqlx/cmd/join_using_format_test.go +++ b/cmd/gosqlx/cmd/join_using_format_test.go @@ -37,13 +37,13 @@ func TestFormatCommand_JoinUsing_Issue603(t *testing.T) { { in: "SELECT * FROM a JOIN b USING (id)", wantCompact: "SELECT * FROM a INNER JOIN b USING (id)", - wantReadable: "SELECT\n *\nFROM a\nINNER JOIN b USING (id)", + wantReadable: "SELECT *\nFROM a\nINNER JOIN b USING (id)", wantUsing: []string{"id"}, }, { in: "SELECT * FROM a LEFT JOIN b USING (id, k)", wantCompact: "SELECT * FROM a LEFT JOIN b USING (id, k)", - wantReadable: "SELECT\n *\nFROM a\nLEFT JOIN b USING (id, k)", + wantReadable: "SELECT *\nFROM a\nLEFT JOIN b USING (id, k)", wantUsing: []string{"id", "k"}, }, } diff --git a/cmd/gosqlx/cmd/sql_formatter.go b/cmd/gosqlx/cmd/sql_formatter.go index a8e9044d..68e931d8 100644 --- a/cmd/gosqlx/cmd/sql_formatter.go +++ b/cmd/gosqlx/cmd/sql_formatter.go @@ -15,1562 +15,82 @@ package cmd import ( - "fmt" - "os" - "strconv" + "errors" "strings" + pkgformatter "github.com/ajitpratap0/GoSQLX/pkg/formatter" "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" ) -// SQLFormatter provides AST-based SQL formatting with configurable rules +// SQLFormatter renders a parsed AST as SQL text for the format command. +// +// It is a thin adapter over pkg/formatter: every statement is rendered with +// pkgformatter.FormatStatement, so the CLI and the library share a single +// renderer and the CLI has no renderer of its own to fall out of sync (#599). +// Rendering gaps in pkg/formatter are caught, where detectable, by +// verifyFormattedSQL. type SQLFormatter struct { - indent string - newlineLevel int - compact bool - uppercaseKw bool - alignColumns bool - builder strings.Builder + style ast.FormatOptions + compact bool } -// FormatterOptions configures the SQL formatter behavior +// FormatterOptions configures the SQL formatter behavior. type FormatterOptions struct { - Indent string // " " for 2 spaces, "\t" for tab, etc. - Compact bool // Single-line formatting - UppercaseKw bool // Uppercase SQL keywords - AlignColumns bool // Align column lists vertically + Indent string // Indentation unit for readable output: " " for 2 spaces, "\t" for a tab, etc. + Compact bool // Single-line formatting + UppercaseKw bool // Uppercase SQL keywords (lowercase otherwise) + // AlignColumns is retained for compatibility; it is ignored because the + // layout now comes from pkg/formatter. + AlignColumns bool + Dialect string // SQL dialect passed to pkg/formatter (empty for the default) } -// NewSQLFormatter creates a new AST-based SQL formatter +// NewSQLFormatter creates a formatter that renders through pkg/formatter with +// options mapped from opts. func NewSQLFormatter(opts FormatterOptions) *SQLFormatter { - indent := opts.Indent - if indent == "" { - indent = " " // Default to 2 spaces - } - - return &SQLFormatter{ - indent: indent, - compact: opts.Compact, - uppercaseKw: opts.UppercaseKw, - alignColumns: opts.AlignColumns, - } -} - -// Format formats an AST into properly indented SQL -func (f *SQLFormatter) Format(astObj *ast.AST) (string, error) { - f.builder.Reset() - f.newlineLevel = 0 - - for i, stmt := range astObj.Statements { - if i > 0 { - if f.compact { - f.builder.WriteString("; ") - } else { - f.builder.WriteString(";\n\n") - } - } - - if err := f.formatStatement(stmt); err != nil { - return "", fmt.Errorf("failed to format statement: %w", err) - } - } - - return f.builder.String(), nil -} - -// formatStatement formats individual SQL statements -func (f *SQLFormatter) formatStatement(stmt ast.Statement) error { - switch s := stmt.(type) { - case *ast.SelectStatement: - return f.formatSelect(s) - case *ast.InsertStatement: - return f.formatInsert(s) - case *ast.UpdateStatement: - return f.formatUpdate(s) - case *ast.DeleteStatement: - return f.formatDelete(s) - case *ast.CreateTableStatement: - return f.formatCreateTable(s) - case *ast.CreateIndexStatement: - return f.formatCreateIndex(s) - case *ast.AlterTableStatement: //nolint:staticcheck // AlterTableStatement kept for backward compatibility - return f.formatAlterTable(s) - case *ast.AlterStatement: - return f.formatAlterStatement(s) - case *ast.DropStatement: - return f.formatDrop(s) - case *ast.CreateViewStatement: - return f.formatCreateView(s) - case *ast.CreateMaterializedViewStatement: - return f.formatCreateMaterializedView(s) - case *ast.RefreshMaterializedViewStatement: - return f.formatRefreshMaterializedView(s) - case *ast.SetOperation: - return f.formatSetOperation(s) - case *ast.MergeStatement: - return f.formatMergeStatement(s) - default: - return fmt.Errorf("unsupported statement type: %T", stmt) - } -} - -// formatSelect formats SELECT statements with proper indentation and alignment -func (f *SQLFormatter) formatSelect(stmt *ast.SelectStatement) error { - // WITH clause - if stmt.With != nil { - if err := f.formatWithClause(stmt.With); err != nil { - return err - } - f.writeNewline() - } - - // SELECT keyword and columns - f.writeKeyword("SELECT") - if stmt.Distinct { - f.builder.WriteString(" ") - f.writeKeyword("DISTINCT") - } - - if f.compact { - f.builder.WriteString(" ") - f.formatExpressionList(stmt.Columns, ", ") + style := ast.ReadableStyle() + if opts.Compact { + style = ast.CompactStyle() } else { - f.writeNewline() - f.increaseIndent() - f.builder.WriteString(f.currentIndent()) - f.formatExpressionList(stmt.Columns, ",\n"+f.currentIndent()) - f.decreaseIndent() - } - - // FROM clause - if len(stmt.From) > 0 { - f.writeNewline() - f.writeKeyword("FROM") - f.builder.WriteString(" ") - f.formatTableReferences(stmt.From) - } - - // JOINs - for _, join := range stmt.Joins { - join := join // G601: Create local copy to avoid memory aliasing - f.writeNewline() - if err := f.formatJoin(&join); err != nil { - return err - } - } - - // WHERE clause - if stmt.Where != nil { - f.writeNewline() - f.writeKeyword("WHERE") - f.builder.WriteString(" ") - if err := f.formatExpression(stmt.Where); err != nil { - return err - } - } - - // GROUP BY clause - if len(stmt.GroupBy) > 0 { - f.writeNewline() - f.writeKeyword("GROUP BY") - f.builder.WriteString(" ") - f.formatExpressionList(stmt.GroupBy, ", ") - } - - // HAVING clause - if stmt.Having != nil { - f.writeNewline() - f.writeKeyword("HAVING") - f.builder.WriteString(" ") - if err := f.formatExpression(stmt.Having); err != nil { - return err - } - } - - // WINDOW clause: named window definitions referenced by OVER w / - // OVER (w ...). Dropping it would silently change query semantics. - if len(stmt.Windows) > 0 { - f.writeNewline() - f.writeKeyword("WINDOW") - f.builder.WriteString(" ") - for i := range stmt.Windows { - if i > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(stmt.Windows[i].Name) - f.builder.WriteString(" ") - f.writeKeyword("AS") - f.builder.WriteString(" (") - if err := f.formatWindowSpec(&stmt.Windows[i]); err != nil { - return err - } - f.builder.WriteString(")") - } - } - - // ORDER BY clause - if len(stmt.OrderBy) > 0 { - f.writeNewline() - f.writeKeyword("ORDER BY") - f.builder.WriteString(" ") - for i, orderBy := range stmt.OrderBy { - if i > 0 { - f.builder.WriteString(", ") - } - if orderBy.Expression != nil { - if err := f.formatExpression(orderBy.Expression); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to format ORDER BY expression: %v\n", err) - } - } - // Add ASC/DESC - if !orderBy.Ascending { - f.builder.WriteString(" DESC") - } - // Add NULLS FIRST/LAST if specified - if orderBy.NullsFirst != nil { - if *orderBy.NullsFirst { - f.builder.WriteString(" NULLS FIRST") - } else { - f.builder.WriteString(" NULLS LAST") - } - } - } - } - - // LIMIT clause - if stmt.Limit != nil { - f.writeNewline() - f.writeKeyword("LIMIT") - f.builder.WriteString(fmt.Sprintf(" %d", *stmt.Limit)) - } - - // OFFSET clause - if stmt.Offset != nil { - f.writeNewline() - f.writeKeyword("OFFSET") - f.builder.WriteString(fmt.Sprintf(" %d", *stmt.Offset)) - } - - return nil -} - -// formatSetOperation formats UNION, EXCEPT, INTERSECT statements -func (f *SQLFormatter) formatSetOperation(stmt *ast.SetOperation) error { - // Format left statement, in parentheses when it was written that way - if stmt.LeftParenthesized { - f.builder.WriteString("(") - } - if err := f.formatStatement(stmt.Left); err != nil { - return fmt.Errorf("failed to format left side of %s: %w", stmt.Operator, err) - } - if stmt.LeftParenthesized { - f.builder.WriteString(")") - } - - // Add newline and operator - f.writeNewline() - f.writeKeyword(stmt.Operator) - if stmt.All { - f.builder.WriteString(" ") - f.writeKeyword("ALL") - } - f.writeNewline() - - // Format right statement; a right-hand set operation always needs - // parentheses to keep its grouping - rightSetOp, ok := stmt.Right.(*ast.SetOperation) - rightParens := stmt.RightParenthesized || (ok && rightSetOp != nil) - if rightParens { - f.builder.WriteString("(") - } - if err := f.formatStatement(stmt.Right); err != nil { - return fmt.Errorf("failed to format right side of %s: %w", stmt.Operator, err) - } - if rightParens { - f.builder.WriteString(")") - } - - return nil -} - -// formatInsert formats INSERT statements -func (f *SQLFormatter) formatInsert(stmt *ast.InsertStatement) error { - // Leading WITH clause: WITH c AS (...) INSERT INTO ... (#537) - if stmt.With != nil { - if err := f.formatWithClause(stmt.With); err != nil { - return err - } - f.writeNewline() - } - - f.writeKeyword("INSERT INTO") - f.builder.WriteString(" " + stmt.TableName) - - if len(stmt.Columns) > 0 { - f.builder.WriteString(" (") - f.formatExpressionList(stmt.Columns, ", ") - f.builder.WriteString(")") - } - - if stmt.DefaultValues { - f.writeNewline() - f.writeKeyword("DEFAULT VALUES") - } - - if len(stmt.Values) > 0 { - f.writeNewline() - f.writeKeyword("VALUES") - for i, row := range stmt.Values { - if i > 0 { - f.builder.WriteString(",") - } - f.builder.WriteString(" (") - f.formatExpressionList(row, ", ") - f.builder.WriteString(")") - } - } - - if stmt.Query != nil { - f.writeNewline() - if sel, ok := stmt.Query.(*ast.SelectStatement); ok { - return f.formatSelect(sel) - } - // For SetOperation or other statement types, use Format if available - if fmtable, ok := stmt.Query.(interface { - Format(ast.FormatOptions) string - }); ok { - f.builder.WriteString(fmtable.Format(ast.FormatOptions{})) - } - } - - return nil -} - -// formatUpdate formats UPDATE statements -func (f *SQLFormatter) formatUpdate(stmt *ast.UpdateStatement) error { - // Leading WITH clause: WITH c AS (...) UPDATE ... (#537) - if stmt.With != nil { - if err := f.formatWithClause(stmt.With); err != nil { - return err - } - f.writeNewline() - } - - f.writeKeyword("UPDATE") - f.builder.WriteString(" " + stmt.TableName) - - if stmt.Alias != "" { - f.builder.WriteString(" " + stmt.Alias) - } - - if len(stmt.Assignments) > 0 { - f.writeNewline() - f.writeKeyword("SET") - f.builder.WriteString(" ") - - for i, update := range stmt.Assignments { - update := update // G601: Create local copy to avoid memory aliasing - if i > 0 { - f.builder.WriteString(", ") - } - if err := f.formatUpdateExpression(&update); err != nil { - return err - } - } - } - - // PostgreSQL/SQL Server UPDATE ... FROM source list (#610) - if len(stmt.From) > 0 { - f.writeNewline() - f.writeKeyword("FROM") - f.builder.WriteString(" ") - f.formatTableReferences(stmt.From) - } - - if stmt.Where != nil { - f.writeNewline() - f.writeKeyword("WHERE") - f.builder.WriteString(" ") - if err := f.formatExpression(stmt.Where); err != nil { - return err + indent := opts.Indent + if indent == "" { + indent = " " // Default to 2 spaces } - } - - return nil -} - -// formatDelete formats DELETE statements -func (f *SQLFormatter) formatDelete(stmt *ast.DeleteStatement) error { - // Leading WITH clause: WITH c AS (...) DELETE ... (#537) - if stmt.With != nil { - if err := f.formatWithClause(stmt.With); err != nil { - return err - } - f.writeNewline() - } - - f.writeKeyword("DELETE FROM") - f.builder.WriteString(" " + stmt.TableName) - - if stmt.Alias != "" { - f.builder.WriteString(" " + stmt.Alias) - } - - if stmt.Where != nil { - f.writeNewline() - f.writeKeyword("WHERE") - f.builder.WriteString(" ") - if err := f.formatExpression(stmt.Where); err != nil { - return err - } - } - - return nil -} - -// formatCreateTable formats CREATE TABLE statements -func (f *SQLFormatter) formatCreateTable(stmt *ast.CreateTableStatement) error { - f.writeKeyword("CREATE") - if stmt.Temporary { - f.builder.WriteString(" ") - f.writeKeyword("TEMPORARY") - } - f.builder.WriteString(" ") - f.writeKeyword("TABLE") - - if stmt.IfNotExists { - f.builder.WriteString(" ") - f.writeKeyword("IF NOT EXISTS") - } - - f.builder.WriteString(" " + stmt.Name + " (") - - if !f.compact { - f.writeNewline() - f.increaseIndent() - } - - for i, col := range stmt.Columns { - col := col // G601: Create local copy to avoid memory aliasing - if i > 0 { - f.builder.WriteString(",") - if !f.compact { - f.writeNewline() - } else { - f.builder.WriteString(" ") - } - } - if !f.compact { - f.builder.WriteString(f.currentIndent()) - } - f.formatColumnDef(&col) - } - - if !f.compact { - f.decreaseIndent() - f.writeNewline() - } - f.builder.WriteString(")") - - return nil -} - -// formatCreateIndex formats CREATE INDEX statements -func (f *SQLFormatter) formatCreateIndex(stmt *ast.CreateIndexStatement) error { - f.writeKeyword("CREATE") - if stmt.Unique { - f.builder.WriteString(" ") - f.writeKeyword("UNIQUE") - } - f.builder.WriteString(" ") - f.writeKeyword("INDEX") - - if stmt.IfNotExists { - f.builder.WriteString(" ") - f.writeKeyword("IF NOT EXISTS") - } - - f.builder.WriteString(" " + stmt.Name) - f.builder.WriteString(" ") - f.writeKeyword("ON") - f.builder.WriteString(" " + stmt.Table + " (") - - for i, col := range stmt.Columns { - if i > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(col.Column) - if col.Direction != "" { - f.builder.WriteString(" " + col.Direction) - } - } - f.builder.WriteString(")") - - return nil -} - -// formatAlterTable formats ALTER TABLE statements -func (f *SQLFormatter) formatAlterTable(stmt *ast.AlterTableStatement) error { //nolint:staticcheck // AlterTableStatement kept for backward compatibility - f.writeKeyword("ALTER TABLE") - f.builder.WriteString(" " + stmt.Table) - - for i, action := range stmt.Actions { - action := action // G601: Create local copy to avoid memory aliasing - if i > 0 { - f.builder.WriteString(",") - } - f.writeNewline() - f.increaseIndent() - f.builder.WriteString(f.currentIndent()) - f.formatAlterTableAction(&action) - f.decreaseIndent() - } - - return nil -} - -// formatAlterStatement formats the generic ALTER statement (from alter.go) -func (f *SQLFormatter) formatAlterStatement(stmt *ast.AlterStatement) error { - // Format ALTER TABLE if it's a table alteration - if stmt.Type == ast.AlterTypeTable { - f.writeKeyword("ALTER TABLE") - f.builder.WriteString(" " + stmt.Name) - - // Format the operation if present - if stmt.Operation != nil { - if op, ok := stmt.Operation.(*ast.AlterTableOperation); ok { - f.writeNewline() - f.increaseIndent() - f.builder.WriteString(f.currentIndent()) - f.formatAlterTableOperation(op) - f.decreaseIndent() - } - } - return nil - } - - // For other ALTER types, provide a basic format - f.writeKeyword("ALTER") - switch stmt.Type { - case ast.AlterTypeRole: - f.builder.WriteString(" ") - f.writeKeyword("ROLE") - case ast.AlterTypePolicy: - f.builder.WriteString(" ") - f.writeKeyword("POLICY") - case ast.AlterTypeConnector: - f.builder.WriteString(" ") - f.writeKeyword("CONNECTOR") - } - f.builder.WriteString(" " + stmt.Name) - - return nil -} - -// formatAlterTableOperation formats a single ALTER TABLE operation -func (f *SQLFormatter) formatAlterTableOperation(op *ast.AlterTableOperation) { - switch op.Type { - case ast.AddColumn: - f.writeKeyword("ADD") - if op.ColumnKeyword { - f.builder.WriteString(" ") - f.writeKeyword("COLUMN") - } - if op.ColumnDef != nil { - f.builder.WriteString(" " + op.ColumnDef.Name) - if op.ColumnDef.Type != "" { - f.builder.WriteString(" " + op.ColumnDef.Type) - } - } - case ast.DropColumn: - f.writeKeyword("DROP") - if op.ColumnKeyword { - f.builder.WriteString(" ") - f.writeKeyword("COLUMN") - } - if op.ColumnDef != nil { - f.builder.WriteString(" " + op.ColumnDef.Name) - } - case ast.AddConstraint: - f.writeKeyword("ADD CONSTRAINT") - if op.Constraint != nil && op.Constraint.Name != "" { - f.builder.WriteString(" " + op.Constraint.Name) - } - case ast.DropConstraint: - f.writeKeyword("DROP CONSTRAINT") - if op.ConstraintName != nil { - f.builder.WriteString(" " + op.ConstraintName.String()) - } - case ast.RenameColumn: - f.writeKeyword("RENAME COLUMN") - if op.OldColumnName != nil { - f.builder.WriteString(" " + op.OldColumnName.String()) - } - f.builder.WriteString(" ") - f.writeKeyword("TO") - if op.NewColumnName != nil { - f.builder.WriteString(" " + op.NewColumnName.String()) - } - default: - // Fallback for unsupported operations - f.builder.WriteString("...") - } -} - -// formatWithClause formats WITH (CTE) clauses -func (f *SQLFormatter) formatWithClause(with *ast.WithClause) error { - f.writeKeyword("WITH") - if with.Recursive { - f.builder.WriteString(" ") - f.writeKeyword("RECURSIVE") - } - f.builder.WriteString(" ") - - for i, cte := range with.CTEs { - if i > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(cte.Name) - - if len(cte.Columns) > 0 { - f.builder.WriteString(" (") - for j, col := range cte.Columns { - if j > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(col) - } - f.builder.WriteString(")") - } - - f.builder.WriteString(" ") - f.writeKeyword("AS") - f.builder.WriteString(" (") - - if !f.compact { - f.writeNewline() - f.increaseIndent() - } - - if err := f.formatStatement(cte.Statement); err != nil { - return err - } - - if !f.compact { - f.decreaseIndent() - f.writeNewline() - } - f.builder.WriteString(")") - } - - return nil -} - -// formatJoin formats JOIN clauses -func (f *SQLFormatter) formatJoin(join *ast.JoinClause) error { - if join.Type != "" { - f.writeKeyword(join.Type) - f.builder.WriteString(" ") - } - f.writeKeyword("JOIN") - f.builder.WriteString(" ") - f.formatTableReference(&join.Right) - - if len(join.Using) > 0 { - f.builder.WriteString(" ") - f.writeKeyword("USING") - f.builder.WriteString(" (") - for i, u := range join.Using { - if i > 0 { - f.builder.WriteString(", ") - } - if err := f.formatExpression(u); err != nil { - return err - } - } - f.builder.WriteString(")") - } else if join.Condition != nil { - f.builder.WriteString(" ") - f.writeKeyword("ON") - f.builder.WriteString(" ") - if err := f.formatExpression(join.Condition); err != nil { - return err - } - } - - return nil -} - -// formatExpression formats SQL expressions -func (f *SQLFormatter) formatExpression(expr ast.Expression) error { - switch e := expr.(type) { - case *ast.Identifier: - if e.Table != "" { - f.builder.WriteString(e.Table + ".") - } - f.builder.WriteString(e.Name) - case *ast.LiteralValue: - // Handle string literals with proper quoting - switch e.Type { - case "string", "STRING": - // Escape single quotes in the string value and wrap in quotes - // Use type assertion for efficiency instead of fmt.Sprintf - var strVal string - if str, ok := e.Value.(string); ok { - strVal = str - } else { - strVal = fmt.Sprintf("%v", e.Value) - } - escaped := strings.ReplaceAll(strVal, "'", "''") - f.builder.WriteString("'") - f.builder.WriteString(escaped) - f.builder.WriteString("'") - case "null", "NULL": - f.writeKeyword("NULL") - default: - // For non-string types, use type assertions for common types - switch v := e.Value.(type) { - case string: - f.builder.WriteString(v) - case int: - f.builder.WriteString(strconv.Itoa(v)) - case int64: - f.builder.WriteString(strconv.FormatInt(v, 10)) - case float64: - f.builder.WriteString(strconv.FormatFloat(v, 'f', -1, 64)) - case bool: - if v { - f.writeKeyword("TRUE") - } else { - f.writeKeyword("FALSE") - } - default: - f.builder.WriteString(fmt.Sprintf("%v", e.Value)) - } - } - case *ast.BinaryExpression: - // NOT EXISTS (subquery) is parsed as a NOT with the EXISTS on the left and no right operand. - if e.Operator == "NOT" && e.Right == nil { - f.writeKeyword("NOT") - f.builder.WriteString(" ") - return f.formatExpression(e.Left) - } - // Handle IS NULL / IS NOT NULL specially - if e.Operator == "IS NULL" { - if err := f.formatExpression(e.Left); err != nil { - return err - } - if e.Not { - f.builder.WriteString(" IS NOT NULL") - } else { - f.builder.WriteString(" IS NULL") - } - return nil - } - // Handle LIKE operator - if e.Operator == "LIKE" { - if err := f.formatExpression(e.Left); err != nil { - return err - } - if e.Not { - f.builder.WriteString(" NOT LIKE ") - } else { - f.builder.WriteString(" LIKE ") - } - if err := f.formatExpression(e.Right); err != nil { - return err - } - return nil - } - // Standard binary expression - if err := f.formatExpression(e.Left); err != nil { - return err - } - f.builder.WriteString(" " + e.Operator + " ") - if err := f.formatExpression(e.Right); err != nil { - return err - } - case *ast.FunctionCall: - f.builder.WriteString(e.Name) - if len(e.Parameters) > 0 { - // ClickHouse parametric aggregates: quantile(0.5)(x). - // Parameters render in their own parenthesis group before args. - f.builder.WriteString("(") - f.formatExpressionList(e.Parameters, ", ") - f.builder.WriteString(")") - } - f.builder.WriteString("(") - if e.Distinct { - f.writeKeyword("DISTINCT") - f.builder.WriteString(" ") - } - f.formatExpressionList(e.Arguments, ", ") - - // ORDER BY inside aggregate functions (STRING_AGG, ARRAY_AGG, etc.) - if len(e.OrderBy) > 0 { - f.builder.WriteString(" ") - f.writeKeyword("ORDER BY") - f.builder.WriteString(" ") - for i, orderBy := range e.OrderBy { - if i > 0 { - f.builder.WriteString(", ") - } - if err := f.formatExpression(orderBy.Expression); err != nil { - return err - } - if !orderBy.Ascending { - f.builder.WriteString(" ") - f.writeKeyword("DESC") - } - if orderBy.NullsFirst != nil { - f.builder.WriteString(" ") - f.writeKeyword("NULLS") - f.builder.WriteString(" ") - if *orderBy.NullsFirst { - f.writeKeyword("FIRST") - } else { - f.writeKeyword("LAST") - } - } - } - } - - f.builder.WriteString(")") - - // Filter clause (SQL:2003 T612) - if e.Filter != nil { - f.builder.WriteString(" ") - f.writeKeyword("FILTER") - f.builder.WriteString(" (") - f.writeKeyword("WHERE") - f.builder.WriteString(" ") - if err := f.formatExpression(e.Filter); err != nil { - return err - } - f.builder.WriteString(")") - } - - // Window function (OVER clause) - if o := e.Over; o != nil { - f.builder.WriteString(" ") - f.writeKeyword("OVER") - f.builder.WriteString(" ") - if o.Name != "" && o.BaseWindow == "" && len(o.PartitionBy) == 0 && len(o.OrderBy) == 0 && o.FrameClause == nil { - // Bare named-window reference: OVER w. - f.builder.WriteString(o.Name) - } else { - // A hand-built spec may carry the referenced window in Name - // alongside other parts; render it as the base window, matching - // ast.FunctionCall.SQL(). - spec := o - if o.BaseWindow == "" && o.Name != "" { - cp := *o - cp.BaseWindow = o.Name - spec = &cp - } - f.builder.WriteString("(") - if err := f.formatWindowSpec(spec); err != nil { - return err - } - f.builder.WriteString(")") - } - } - case *ast.CaseExpression: - f.writeKeyword("CASE") - if e.Value != nil { - f.builder.WriteString(" ") - if err := f.formatExpression(e.Value); err != nil { - return err - } - } - - for _, when := range e.WhenClauses { - f.builder.WriteString(" ") - f.writeKeyword("WHEN") - f.builder.WriteString(" ") - if err := f.formatExpression(when.Condition); err != nil { - return err - } - f.builder.WriteString(" ") - f.writeKeyword("THEN") - f.builder.WriteString(" ") - if err := f.formatExpression(when.Result); err != nil { - return err - } - } - - if e.ElseClause != nil { - f.builder.WriteString(" ") - f.writeKeyword("ELSE") - f.builder.WriteString(" ") - if err := f.formatExpression(e.ElseClause); err != nil { - return err - } - } - f.builder.WriteString(" ") - f.writeKeyword("END") - case *ast.BetweenExpression: - // Handle BETWEEN expr AND expr - if err := f.formatExpression(e.Expr); err != nil { - return err - } - if e.Not { - f.builder.WriteString(" ") - f.writeKeyword("NOT BETWEEN") + if strings.Contains(indent, "\t") { + style.IndentStyle = ast.IndentTabs + style.IndentWidth = strings.Count(indent, "\t") } else { - f.builder.WriteString(" ") - f.writeKeyword("BETWEEN") - } - f.builder.WriteString(" ") - if err := f.formatExpression(e.Lower); err != nil { - return err - } - f.builder.WriteString(" ") - f.writeKeyword("AND") - f.builder.WriteString(" ") - if err := f.formatExpression(e.Upper); err != nil { - return err - } - case *ast.InExpression: - // Handle IN (values) or IN (subquery) - if err := f.formatExpression(e.Expr); err != nil { - return err - } - if e.Not { - f.builder.WriteString(" ") - f.writeKeyword("NOT IN") - } else { - f.builder.WriteString(" ") - f.writeKeyword("IN") - } - f.builder.WriteString(" (") - if e.Subquery != nil { - // IN (SELECT ...) - if selectStmt, ok := e.Subquery.(*ast.SelectStatement); ok { - if err := f.formatSelect(selectStmt); err != nil { - return err - } - } - } else { - // IN (value1, value2, ...) - f.formatExpressionList(e.List, ", ") - } - f.builder.WriteString(")") - case *ast.ExistsExpression: - // Handle EXISTS (subquery) - NOT EXISTS is handled via UnaryExpression - f.writeKeyword("EXISTS") - f.builder.WriteString(" (") - if selectStmt, ok := e.Subquery.(*ast.SelectStatement); ok { - if err := f.formatSelect(selectStmt); err != nil { - return err - } - } - f.builder.WriteString(")") - case *ast.SubqueryExpression: - // Handle scalar subquery (SELECT ...) - f.builder.WriteString("(") - if selectStmt, ok := e.Subquery.(*ast.SelectStatement); ok { - if err := f.formatSelect(selectStmt); err != nil { - return err - } - } - f.builder.WriteString(")") - case *ast.UnaryExpression: - // Handle NOT expr, - expr, etc. - f.builder.WriteString(e.Operator.String()) - f.builder.WriteString(" ") - if err := f.formatExpression(e.Expr); err != nil { - return err - } - case *ast.ParenthesizedExpression: - // Preserve grouping parentheses, e.g. (a OR b) AND c (see #519) - f.builder.WriteString("(") - if err := f.formatExpression(e.Expr); err != nil { - return err - } - f.builder.WriteString(")") - case *ast.AliasedExpression: - // Handle expr AS alias - if err := f.formatExpression(e.Expr); err != nil { - return err - } - f.builder.WriteString(" ") - f.writeKeyword("AS") - f.builder.WriteString(" ") - // Quote alias if it contains special characters or is a reserved keyword - f.formatIdentifier(e.Alias) - default: - // Fallback for unsupported expressions - f.builder.WriteString(expr.TokenLiteral()) - } - - return nil -} - -// formatWindowSpec formats window specifications for window functions -func (f *SQLFormatter) formatWindowSpec(spec *ast.WindowSpec) error { - // The existing window this spec refines (OVER (w ORDER BY z), - // WINDOW w2 AS (w1 ...)) comes first inside the parentheses. - hasBase := spec.BaseWindow != "" - if hasBase { - f.builder.WriteString(spec.BaseWindow) - } - - if len(spec.PartitionBy) > 0 { - if hasBase { - f.builder.WriteString(" ") - } - f.writeKeyword("PARTITION BY") - f.builder.WriteString(" ") - f.formatExpressionList(spec.PartitionBy, ", ") - } - - if len(spec.OrderBy) > 0 { - if hasBase || len(spec.PartitionBy) > 0 { - f.builder.WriteString(" ") - } - f.writeKeyword("ORDER BY") - f.builder.WriteString(" ") - for i, orderBy := range spec.OrderBy { - if i > 0 { - f.builder.WriteString(", ") - } - if orderBy.Expression != nil { - if err := f.formatExpression(orderBy.Expression); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to format window ORDER BY expression: %v\n", err) - } - } - if !orderBy.Ascending { - f.builder.WriteString(" DESC") - } - if orderBy.NullsFirst != nil { - if *orderBy.NullsFirst { - f.builder.WriteString(" NULLS FIRST") - } else { - f.builder.WriteString(" NULLS LAST") - } - } - } - } - - if fc := spec.FrameClause; fc != nil { - if hasBase || len(spec.PartitionBy) > 0 || len(spec.OrderBy) > 0 { - f.builder.WriteString(" ") - } - f.writeKeyword(fc.Type) - f.builder.WriteString(" ") - // A numeric bound keeps its offset in Value (e.g. Value=2, - // Type="PRECEDING"); CURRENT ROW and the UNBOUNDED forms carry no - // Value. BETWEEN is only emitted when there are two bounds. - writeBound := func(b *ast.WindowFrameBound) error { - if b.Value != nil { - if err := f.formatExpression(b.Value); err != nil { - return err - } - f.builder.WriteString(" ") - } - f.writeKeyword(b.Type) - return nil - } - if fc.End != nil { - f.writeKeyword("BETWEEN") - f.builder.WriteString(" ") - if err := writeBound(&fc.Start); err != nil { - return err - } - f.builder.WriteString(" ") - f.writeKeyword("AND") - f.builder.WriteString(" ") - if err := writeBound(fc.End); err != nil { - return err - } - } else if err := writeBound(&fc.Start); err != nil { - return err - } - if fc.Exclude != "" { - f.builder.WriteString(" ") - f.writeKeyword("EXCLUDE") - f.builder.WriteString(" ") - f.writeKeyword(fc.Exclude) - } - } - - return nil -} - -// Helper methods for formatting -func (f *SQLFormatter) formatExpressionList(exprs []ast.Expression, separator string) { - for i, expr := range exprs { - if i > 0 { - f.builder.WriteString(separator) - } - if err := f.formatExpression(expr); err != nil { - // Log error but continue formatting to avoid breaking output - fmt.Fprintf(os.Stderr, "Warning: failed to format expression: %v\n", err) - } - } -} - -func (f *SQLFormatter) formatTableReferences(tables []ast.TableReference) { - for i, table := range tables { - table := table // G601: Create local copy to avoid memory aliasing - if i > 0 { - f.builder.WriteString(", ") + style.IndentWidth = len(indent) } - f.formatTableReference(&table) - } -} - -func (f *SQLFormatter) formatTableReference(table *ast.TableReference) { - // Output LATERAL keyword if set - if table.Lateral { - f.writeKeyword("LATERAL") - f.builder.WriteString(" ") } - - if table.Subquery != nil { - // Format derived table (subquery) - f.builder.WriteString("(") - if err := f.formatSelect(table.Subquery); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to format derived table: %v\n", err) - } - f.builder.WriteString(")") - } else if table.SetSubquery != nil { - // Format derived table whose body is a set operation - f.builder.WriteString("(") - if err := f.formatSetOperation(table.SetSubquery); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to format derived table: %v\n", err) - } - f.builder.WriteString(")") + if opts.UppercaseKw { + style.KeywordCase = ast.KeywordUpper } else { - // Format regular table name - f.builder.WriteString(table.Name) - } - if table.Alias != "" { - f.builder.WriteString(" ") - f.writeKeyword("AS") - f.builder.WriteString(" " + table.Alias) + style.KeywordCase = ast.KeywordLower } + // Statement separators are added by Format; pkg/formatter's readable + // semicolons would also be emitted inside subqueries. + style.AddSemicolon = false + style.Dialect = opts.Dialect + return &SQLFormatter{style: style, compact: opts.Compact} } -func (f *SQLFormatter) formatUpdateExpression(update *ast.UpdateExpression) error { - if err := f.formatExpression(update.Column); err != nil { - return err - } - f.builder.WriteString(" = ") - return f.formatExpression(update.Value) -} - -func (f *SQLFormatter) formatColumnDef(col *ast.ColumnDef) { - f.builder.WriteString(col.Name + " " + col.Type) - for _, constraint := range col.Constraints { - f.builder.WriteString(" ") - f.writeKeyword(constraint.Type) - // Format DEFAULT value if present - if constraint.Type == "DEFAULT" && constraint.Default != nil { - f.builder.WriteString(" ") - if err := f.formatExpression(constraint.Default); err != nil { - // Fallback to token literal on error - f.builder.WriteString(constraint.Default.TokenLiteral()) - } - } - // Format CHECK expression if present - if constraint.Type == "CHECK" && constraint.Check != nil { - f.builder.WriteString(" (") - if err := f.formatExpression(constraint.Check); err != nil { - f.builder.WriteString(constraint.Check.TokenLiteral()) - } - f.builder.WriteString(")") - } - // Format REFERENCES if present - if constraint.Type == "REFERENCES" && constraint.References != nil { - f.builder.WriteString(" ") - f.builder.WriteString(constraint.References.Table) - if len(constraint.References.Columns) > 0 { - f.builder.WriteString("(") - f.builder.WriteString(strings.Join(constraint.References.Columns, ", ")) - f.builder.WriteString(")") - } - if constraint.References.OnDelete != "" { - f.builder.WriteString(" ") - f.writeKeyword("ON DELETE") - f.builder.WriteString(" ") - f.writeKeyword(constraint.References.OnDelete) - } - if constraint.References.OnUpdate != "" { - f.builder.WriteString(" ") - f.writeKeyword("ON UPDATE") - f.builder.WriteString(" ") - f.writeKeyword(constraint.References.OnUpdate) - } - } - } -} - -func (f *SQLFormatter) formatAlterTableAction(action *ast.AlterTableAction) { - f.builder.WriteString(action.Type) - if action.ColumnName != "" { - f.builder.WriteString(" " + action.ColumnName) - } - if action.ColumnDef != nil { - f.builder.WriteString(" ") - f.formatColumnDef(action.ColumnDef) - } -} - -// Indentation and formatting helpers -func (f *SQLFormatter) writeKeyword(keyword string) { - if f.uppercaseKw { - f.builder.WriteString(strings.ToUpper(keyword)) - } else { - f.builder.WriteString(strings.ToLower(keyword)) - } -} - -// formatIdentifier formats an identifier, quoting it if it contains special characters -// or is a reserved keyword -func (f *SQLFormatter) formatIdentifier(ident string) { - if f.needsQuoting(ident) { - f.builder.WriteString("\"") - // Escape any existing double quotes by doubling them - escaped := strings.ReplaceAll(ident, "\"", "\"\"") - f.builder.WriteString(escaped) - f.builder.WriteString("\"") - } else { - f.builder.WriteString(ident) - } -} - -// needsQuoting returns true if the identifier needs to be quoted -func (f *SQLFormatter) needsQuoting(ident string) bool { - if len(ident) == 0 { - return true - } - // Check if it starts with a digit - if ident[0] >= '0' && ident[0] <= '9' { - return true - } - // Check for special characters (allow only letters, digits, and underscore) - for _, c := range ident { - if (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') && (c < '0' || c > '9') && c != '_' { - return true - } - } - // Check for common SQL reserved keywords that might be used as aliases - reserved := map[string]bool{ - "SELECT": true, "FROM": true, "WHERE": true, "AND": true, "OR": true, - "ORDER": true, "BY": true, "GROUP": true, "HAVING": true, "JOIN": true, - "LEFT": true, "RIGHT": true, "INNER": true, "OUTER": true, "ON": true, - "AS": true, "TABLE": true, "INDEX": true, "CREATE": true, "DROP": true, - "INSERT": true, "UPDATE": true, "DELETE": true, "INTO": true, "VALUES": true, - "SET": true, "NULL": true, "NOT": true, "IN": true, "LIKE": true, - "BETWEEN": true, "EXISTS": true, "CASE": true, "WHEN": true, "THEN": true, - "ELSE": true, "END": true, "DISTINCT": true, "ALL": true, "UNION": true, - } - return reserved[strings.ToUpper(ident)] -} - -func (f *SQLFormatter) writeNewline() { - if !f.compact { - f.builder.WriteString("\n") - } else { - f.builder.WriteString(" ") - } -} - -func (f *SQLFormatter) increaseIndent() { - f.newlineLevel++ -} - -func (f *SQLFormatter) decreaseIndent() { - if f.newlineLevel > 0 { - f.newlineLevel-- +// Format renders every statement of astObj, separated by "; " in compact mode +// and ";\n\n" otherwise. No trailing semicolon is added. +func (f *SQLFormatter) Format(astObj *ast.AST) (string, error) { + if astObj == nil { + return "", errors.New("cannot format a nil AST") } -} - -func (f *SQLFormatter) currentIndent() string { + sep := ";\n\n" if f.compact { - return "" - } - return strings.Repeat(f.indent, f.newlineLevel) -} - -// formatDrop formats DROP statements -func (f *SQLFormatter) formatDrop(stmt *ast.DropStatement) error { - f.writeKeyword("DROP") - f.builder.WriteString(" ") - f.writeKeyword(stmt.ObjectType) - - if stmt.IfExists { - f.builder.WriteString(" ") - f.writeKeyword("IF EXISTS") + sep = "; " } - - for i, name := range stmt.Names { + var sb strings.Builder + for i, stmt := range astObj.Statements { if i > 0 { - f.builder.WriteString(",") + sb.WriteString(sep) } - f.builder.WriteString(" " + name) - } - - if stmt.CascadeType != "" { - f.builder.WriteString(" ") - f.writeKeyword(stmt.CascadeType) + sb.WriteString(pkgformatter.FormatStatement(stmt, f.style)) } - - return nil -} - -// formatCreateView formats CREATE VIEW statements -func (f *SQLFormatter) formatCreateView(stmt *ast.CreateViewStatement) error { - f.writeKeyword("CREATE") - if stmt.OrReplace { - f.builder.WriteString(" ") - f.writeKeyword("OR REPLACE") - } - if stmt.Temporary { - f.builder.WriteString(" ") - f.writeKeyword("TEMPORARY") - } - f.builder.WriteString(" ") - f.writeKeyword("VIEW") - - if stmt.IfNotExists { - f.builder.WriteString(" ") - f.writeKeyword("IF NOT EXISTS") - } - - f.builder.WriteString(" " + stmt.Name) - - if len(stmt.Columns) > 0 { - f.builder.WriteString(" (") - for i, col := range stmt.Columns { - if i > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(col) - } - f.builder.WriteString(")") - } - - f.builder.WriteString(" ") - f.writeKeyword("AS") - f.writeNewline() - f.increaseIndent() - if err := f.formatStatement(stmt.Query); err != nil { - return err - } - f.decreaseIndent() - - return nil -} - -// formatCreateMaterializedView formats CREATE MATERIALIZED VIEW statements -func (f *SQLFormatter) formatCreateMaterializedView(stmt *ast.CreateMaterializedViewStatement) error { - f.writeKeyword("CREATE MATERIALIZED VIEW") - - if stmt.IfNotExists { - f.builder.WriteString(" ") - f.writeKeyword("IF NOT EXISTS") - } - - f.builder.WriteString(" " + stmt.Name) - - if len(stmt.Columns) > 0 { - f.builder.WriteString(" (") - for i, col := range stmt.Columns { - if i > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(col) - } - f.builder.WriteString(")") - } - - f.builder.WriteString(" ") - f.writeKeyword("AS") - f.writeNewline() - f.increaseIndent() - if err := f.formatStatement(stmt.Query); err != nil { - return err - } - f.decreaseIndent() - - if stmt.WithData != nil { - f.writeNewline() - if *stmt.WithData { - f.writeKeyword("WITH DATA") - } else { - f.writeKeyword("WITH NO DATA") - } - } - - return nil -} - -// formatRefreshMaterializedView formats REFRESH MATERIALIZED VIEW statements -func (f *SQLFormatter) formatRefreshMaterializedView(stmt *ast.RefreshMaterializedViewStatement) error { - f.writeKeyword("REFRESH MATERIALIZED VIEW") - - if stmt.Concurrently { - f.builder.WriteString(" ") - f.writeKeyword("CONCURRENTLY") - } - - f.builder.WriteString(" " + stmt.Name) - - if stmt.WithData != nil { - f.builder.WriteString(" ") - if *stmt.WithData { - f.writeKeyword("WITH DATA") - } else { - f.writeKeyword("WITH NO DATA") - } - } - - return nil -} - -// formatMergeStatement formats MERGE statements with proper indentation -func (f *SQLFormatter) formatMergeStatement(stmt *ast.MergeStatement) error { - // MERGE INTO target_table - f.writeKeyword("MERGE INTO") - f.builder.WriteString(" ") - f.formatTableReference(&stmt.TargetTable) - if stmt.TargetAlias != "" { - f.builder.WriteString(" " + stmt.TargetAlias) - } - - // USING source_table - f.writeNewline() - f.writeKeyword("USING") - f.builder.WriteString(" ") - f.formatTableReference(&stmt.SourceTable) - if stmt.SourceAlias != "" { - f.builder.WriteString(" " + stmt.SourceAlias) - } - - // ON condition - if stmt.OnCondition != nil { - f.writeNewline() - f.writeKeyword("ON") - f.builder.WriteString(" ") - if err := f.formatExpression(stmt.OnCondition); err != nil { - return fmt.Errorf("failed to format ON condition: %w", err) - } - } - - // WHEN clauses - for _, whenClause := range stmt.WhenClauses { - if err := f.formatMergeWhenClause(whenClause); err != nil { - return fmt.Errorf("failed to format WHEN clause: %w", err) - } - } - - return nil -} - -// formatMergeWhenClause formats a WHEN clause in a MERGE statement -func (f *SQLFormatter) formatMergeWhenClause(when *ast.MergeWhenClause) error { - f.writeNewline() - f.writeKeyword("WHEN") - f.builder.WriteString(" ") - - // Format the match type - switch when.Type { - case "MATCHED": - f.writeKeyword("MATCHED") - case "NOT_MATCHED": - f.writeKeyword("NOT MATCHED") - case "NOT_MATCHED_BY_SOURCE": - f.writeKeyword("NOT MATCHED BY SOURCE") - default: - f.writeKeyword(when.Type) - } - - // Format optional AND condition - if when.Condition != nil { - f.builder.WriteString(" ") - f.writeKeyword("AND") - f.builder.WriteString(" ") - if err := f.formatExpression(when.Condition); err != nil { - return fmt.Errorf("failed to format WHEN condition: %w", err) - } - } - - // Format THEN action - f.builder.WriteString(" ") - f.writeKeyword("THEN") - f.writeNewline() - f.increaseIndent() - f.builder.WriteString(f.currentIndent()) - if err := f.formatMergeAction(when.Action); err != nil { - return fmt.Errorf("failed to format MERGE action: %w", err) - } - f.decreaseIndent() - - return nil -} - -// formatMergeAction formats the action part of a WHEN clause (UPDATE/INSERT/DELETE) -func (f *SQLFormatter) formatMergeAction(action *ast.MergeAction) error { - if action == nil { - return nil - } - - switch action.ActionType { - case "UPDATE": - f.writeKeyword("UPDATE SET") - if len(action.SetClauses) > 0 { - f.builder.WriteString(" ") - for i, setClause := range action.SetClauses { - if i > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(setClause.Column) - f.builder.WriteString(" = ") - if err := f.formatExpression(setClause.Value); err != nil { - return fmt.Errorf("failed to format SET clause value: %w", err) - } - } - } - - case "INSERT": - f.writeKeyword("INSERT") - if action.DefaultValues { - f.builder.WriteString(" ") - f.writeKeyword("DEFAULT VALUES") - } else if len(action.Columns) > 0 { - f.builder.WriteString(" (") - for i, col := range action.Columns { - if i > 0 { - f.builder.WriteString(", ") - } - f.builder.WriteString(col) - } - f.builder.WriteString(")") - if len(action.Values) > 0 { - f.writeNewline() - f.builder.WriteString(f.currentIndent()) - f.writeKeyword("VALUES") - f.builder.WriteString(" (") - f.formatExpressionList(action.Values, ", ") - f.builder.WriteString(")") - } - } else if len(action.Values) > 0 { - f.builder.WriteString(" ") - f.writeKeyword("VALUES") - f.builder.WriteString(" (") - f.formatExpressionList(action.Values, ", ") - f.builder.WriteString(")") - } - - case "DELETE": - f.writeKeyword("DELETE") - - default: - return fmt.Errorf("unsupported merge action type: %s", action.ActionType) - } - - return nil + return sb.String(), nil } diff --git a/cmd/gosqlx/cmd/sql_formatter_test.go b/cmd/gosqlx/cmd/sql_formatter_test.go index 52108a10..ad905c1f 100644 --- a/cmd/gosqlx/cmd/sql_formatter_test.go +++ b/cmd/gosqlx/cmd/sql_formatter_test.go @@ -644,8 +644,11 @@ func TestSQLFormatter_NotExistsDoesNotPanic(t *testing.T) { if err != nil { t.Fatalf("Formatting failed: %v", err) } + if output == "" { + t.Fatal("expected non-empty output") + } if !strings.Contains(output, "NOT EXISTS (") { - t.Errorf("expected NOT EXISTS in output, got:\n%s", output) + t.Errorf("NOT EXISTS not rendered as a prefix NOT (#601): %s", output) } } diff --git a/pkg/formatter/render.go b/pkg/formatter/render.go index a0bd0212..7a5cd2ab 100644 --- a/pkg/formatter/render.go +++ b/pkg/formatter/render.go @@ -140,6 +140,8 @@ func FormatStatement(s ast.Statement, opts ast.FormatOptions) string { return renderSetOperation(v, opts) case *ast.AlterTableStatement: //nolint:staticcheck // AlterTableStatement kept for backward compatibility return renderAlterTable(v, opts) + case *ast.AlterStatement: + return renderAlterStatement(v, opts) case *ast.CreateIndexStatement: return renderCreateIndex(v, opts) case *ast.CreateViewStatement: diff --git a/pkg/formatter/render_alter.go b/pkg/formatter/render_alter.go new file mode 100644 index 00000000..8226c873 --- /dev/null +++ b/pkg/formatter/render_alter.go @@ -0,0 +1,115 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package formatter + +import "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + +// renderAlterStatement renders an *ast.AlterStatement. ALTER TABLE operations +// produced by the parser (ADD COLUMN, ADD CONSTRAINT, DROP COLUMN, DROP +// CONSTRAINT, RENAME TO, RENAME COLUMN, ALTER COLUMN) are rendered in full; +// other ALTER kinds (ROLE, POLICY, CONNECTOR) keep the previous stmtSQL output. +func renderAlterStatement(a *ast.AlterStatement, opts ast.FormatOptions) string { + if a == nil { + return "" + } + if a.Type != ast.AlterTypeTable { + return stmtSQL(a) + } + f := newNodeFormatter(opts) + f.sb.WriteString(f.kw("ALTER TABLE")) + f.sb.WriteString(" ") + f.sb.WriteString(a.Name) + if op, ok := a.Operation.(*ast.AlterTableOperation); ok && op != nil { + f.sb.WriteString(f.clauseSep()) + writeAlterTableOperation(f, op) + } + if opts.AddSemicolon { + f.sb.WriteString(";") + } + return f.result() +} + +// alterIdentName returns the name of i, or "" for nil. +func alterIdentName(i *ast.Ident) string { + if i == nil { + return "" + } + return i.Name +} + +// writeAlterTableOperation renders a single ALTER TABLE operation. Operation +// types the ALTER TABLE parser does not produce are left empty. +func writeAlterTableOperation(f *nodeFormatter, op *ast.AlterTableOperation) { + sb := f.sb + switch op.Type { + case ast.AddColumn: + sb.WriteString(f.kw("ADD COLUMN")) + if op.ColumnDef != nil { + sb.WriteString(" ") + sb.WriteString(columnDefSQL(op.ColumnDef)) + } + case ast.AddConstraint: + sb.WriteString(f.kw("ADD")) + if op.Constraint != nil { + // tableConstraintSQL prints "CONSTRAINT name" itself when named. + if op.Constraint.Name == "" { + sb.WriteString(" ") + sb.WriteString(f.kw("CONSTRAINT")) + } + sb.WriteString(" ") + sb.WriteString(tableConstraintSQL(op.Constraint)) + } + case ast.DropColumn: + sb.WriteString(f.kw("DROP COLUMN")) + sb.WriteString(" ") + sb.WriteString(alterIdentName(op.ColumnName)) + if op.CascadeDrops { + sb.WriteString(" ") + sb.WriteString(f.kw("CASCADE")) + } + case ast.DropConstraint: + sb.WriteString(f.kw("DROP CONSTRAINT")) + sb.WriteString(" ") + sb.WriteString(alterIdentName(op.ConstraintName)) + if op.CascadeDrops { + sb.WriteString(" ") + sb.WriteString(f.kw("CASCADE")) + } + case ast.RenameTable: + sb.WriteString(f.kw("RENAME TO")) + sb.WriteString(" ") + sb.WriteString(op.NewTableName.String()) + case ast.RenameColumn: + oldName := op.ColumnName + if oldName == nil { + oldName = op.OldColumnName + } + sb.WriteString(f.kw("RENAME COLUMN")) + sb.WriteString(" ") + sb.WriteString(alterIdentName(oldName)) + sb.WriteString(" ") + sb.WriteString(f.kw("TO")) + sb.WriteString(" ") + sb.WriteString(alterIdentName(op.NewColumnName)) + case ast.AlterColumn: + sb.WriteString(f.kw("ALTER COLUMN")) + sb.WriteString(" ") + sb.WriteString(alterIdentName(op.ColumnName)) + if op.ColumnDef != nil { + sb.WriteString(" ") + sb.WriteString(columnDefSQL(op.ColumnDef)) + } + } +} diff --git a/pkg/formatter/render_alter_test.go b/pkg/formatter/render_alter_test.go new file mode 100644 index 00000000..73043100 --- /dev/null +++ b/pkg/formatter/render_alter_test.go @@ -0,0 +1,170 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package formatter_test + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" +) + +// parseSingleAlter parses sql and returns its only statement, which must be an +// *ast.AlterStatement. +func parseSingleAlter(t *testing.T, sql string) *ast.AlterStatement { + t.Helper() + tree, err := gosqlx.Parse(sql) + if err != nil { + t.Fatalf("parse %q: %v", sql, err) + } + if len(tree.Statements) != 1 { + t.Fatalf("parse %q: expected 1 statement, got %d", sql, len(tree.Statements)) + } + stmt, ok := tree.Statements[0].(*ast.AlterStatement) + if !ok { + t.Fatalf("parse %q: expected *ast.AlterStatement, got %T", sql, tree.Statements[0]) + } + return stmt +} + +// TestFormatStatement_AlterTable verifies that ALTER TABLE operations parsed +// into *ast.AlterStatement render in full (previously they rendered as a bare +// "ALTER") and that the rendering re-parses to the same statement (#599). +func TestFormatStatement_AlterTable(t *testing.T) { + compactUpper := ast.CompactStyle() + compactUpper.KeywordCase = ast.KeywordUpper + + tests := []struct { + name string + in string + compact string + readable string + }{ + { + name: "add column with constraints", + in: "ALTER TABLE t ADD COLUMN c INT NOT NULL DEFAULT 0", + compact: "ALTER TABLE t ADD COLUMN c INT NOT NULL DEFAULT 0", + readable: "ALTER TABLE t\nADD COLUMN c INT NOT NULL DEFAULT 0;", + }, + { + name: "add named constraint", + in: "ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (id)", + compact: "ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY (id)", + readable: "ALTER TABLE t\nADD CONSTRAINT pk PRIMARY KEY (id);", + }, + { + name: "add unnamed constraint", + in: "ALTER TABLE t ADD CONSTRAINT UNIQUE (a, b)", + compact: "ALTER TABLE t ADD CONSTRAINT UNIQUE (a, b)", + readable: "ALTER TABLE t\nADD CONSTRAINT UNIQUE (a, b);", + }, + { + name: "add foreign key", + in: "ALTER TABLE t ADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES u (id) ON DELETE CASCADE", + compact: "ALTER TABLE t ADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES u (id) ON DELETE CASCADE", + readable: "ALTER TABLE t\nADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES u (id) ON DELETE CASCADE;", + }, + { + name: "drop column", + in: "ALTER TABLE t DROP COLUMN c", + compact: "ALTER TABLE t DROP COLUMN c", + readable: "ALTER TABLE t\nDROP COLUMN c;", + }, + { + name: "drop column cascade", + in: "ALTER TABLE t DROP COLUMN c CASCADE", + compact: "ALTER TABLE t DROP COLUMN c CASCADE", + readable: "ALTER TABLE t\nDROP COLUMN c CASCADE;", + }, + { + name: "drop constraint", + in: "ALTER TABLE t DROP CONSTRAINT pk", + compact: "ALTER TABLE t DROP CONSTRAINT pk", + readable: "ALTER TABLE t\nDROP CONSTRAINT pk;", + }, + { + name: "drop constraint cascade", + in: "ALTER TABLE t DROP CONSTRAINT pk CASCADE", + compact: "ALTER TABLE t DROP CONSTRAINT pk CASCADE", + readable: "ALTER TABLE t\nDROP CONSTRAINT pk CASCADE;", + }, + { + name: "rename table", + in: "ALTER TABLE t RENAME TO u", + compact: "ALTER TABLE t RENAME TO u", + readable: "ALTER TABLE t\nRENAME TO u;", + }, + { + name: "rename column", + in: "ALTER TABLE t RENAME COLUMN a TO b", + compact: "ALTER TABLE t RENAME COLUMN a TO b", + readable: "ALTER TABLE t\nRENAME COLUMN a TO b;", + }, + { + name: "alter column type", + in: "ALTER TABLE t ALTER COLUMN c TYPE BIGINT", + compact: "ALTER TABLE t ALTER COLUMN c TYPE BIGINT", + readable: "ALTER TABLE t\nALTER COLUMN c TYPE BIGINT;", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + stmt := parseSingleAlter(t, tc.in) + + compact := formatter.FormatStatement(stmt, compactUpper) + if compact != tc.compact { + t.Errorf("compact:\n got: %q\nwant: %q", compact, tc.compact) + } + if got := formatter.FormatStatement(stmt, ast.ReadableStyle()); got != tc.readable { + t.Errorf("readable:\n got: %q\nwant: %q", got, tc.readable) + } + + // AlterStatement has no SQL(), so compare renderings of the + // original and the re-parsed statement. + reparsed := parseSingleAlter(t, compact) + if again := formatter.FormatStatement(reparsed, compactUpper); again != compact { + t.Errorf("render is not stable across re-parse:\nfirst: %q\nsecond: %q", compact, again) + } + }) + } +} + +// TestFormatStatement_AlterTable_LowerKeywords checks that keyword casing is +// applied to the ALTER TABLE keywords while identifiers and types are kept. +func TestFormatStatement_AlterTable_LowerKeywords(t *testing.T) { + stmt := parseSingleAlter(t, "ALTER TABLE t ADD COLUMN c INT") + opts := ast.CompactStyle() + opts.KeywordCase = ast.KeywordLower + const want = "alter table t add column c INT" + got := formatter.FormatStatement(stmt, opts) + if got != want { + t.Fatalf("got %q, want %q", got, want) + } + reparsed := parseSingleAlter(t, got) + if again := formatter.FormatStatement(reparsed, opts); again != got { + t.Fatalf("render is not stable across re-parse:\nfirst: %q\nsecond: %q", got, again) + } +} + +// TestFormatStatement_AlterNonTableUnchanged checks that non-table ALTER +// statements keep their previous rendering. +func TestFormatStatement_AlterNonTableUnchanged(t *testing.T) { + stmt := &ast.AlterStatement{Type: ast.AlterTypeRole, Name: "r"} + if got := formatter.FormatStatement(stmt, ast.CompactStyle()); got != stmt.TokenLiteral() { + t.Fatalf("got %q, want token literal %q", got, stmt.TokenLiteral()) + } +} diff --git a/pkg/formatter/unary_minus_test.go b/pkg/formatter/unary_minus_test.go new file mode 100644 index 00000000..7d77ae68 --- /dev/null +++ b/pkg/formatter/unary_minus_test.go @@ -0,0 +1,67 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package formatter_test + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" +) + +// TestFormatStatement_NestedUnaryMinus checks that nested unary minus is not +// rendered as "--", which starts a line comment and silently drops the rest +// of the line, and that the output re-parses to the same query. +func TestFormatStatement_NestedUnaryMinus(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"SELECT - -1 FROM t", "SELECT - -1 FROM t"}, + {"SELECT a FROM t WHERE b = - -1", "SELECT a FROM t WHERE b = - -1"}, + {"SELECT - - -x FROM t", "SELECT - - -x FROM t"}, + {"SELECT -(-x) FROM t", "SELECT -(-x) FROM t"}, + {"SELECT - -x + 1 FROM t", "SELECT - -x + 1 FROM t"}, + } + style := ast.CompactStyle() + style.KeywordCase = ast.KeywordUpper + for _, tc := range tests { + t.Run(tc.in, func(t *testing.T) { + tree, err := gosqlx.Parse(tc.in) + if err != nil { + t.Fatalf("parse %q: %v", tc.in, err) + } + if len(tree.Statements) != 1 { + t.Fatalf("parse %q: expected 1 statement, got %d", tc.in, len(tree.Statements)) + } + got := formatter.FormatStatement(tree.Statements[0], style) + if got != tc.want { + t.Fatalf("FormatStatement(%q) = %q, want %q", tc.in, got, tc.want) + } + if strings.Contains(got, "--") { + t.Fatalf("FormatStatement(%q) = %q contains a line comment marker", tc.in, got) + } + again, err := gosqlx.Parse(got) + if err != nil { + t.Fatalf("output %q does not re-parse: %v", got, err) + } + if a, b := tree.SQL(), again.SQL(); a != b { + t.Fatalf("output changes the query: %q -> %q", a, b) + } + }) + } +} diff --git a/pkg/sql/ast/sql.go b/pkg/sql/ast/sql.go index 0ffc60ac..cb3b9e5a 100644 --- a/pkg/sql/ast/sql.go +++ b/pkg/sql/ast/sql.go @@ -465,7 +465,16 @@ func (u *UnaryExpression) writeSQL(buf *bytes.Buffer) { writeExpr(buf, u.Expr) case Minus: buf.WriteByte('-') + start := buf.Len() writeExpr(buf, u.Expr) + // "--" starts a line comment: separate a nested unary minus or a + // negative literal from this operator ("- -1", not "--1"). + if b := buf.Bytes(); len(b) > start && b[start] == '-' { + operand := append([]byte(nil), b[start:]...) + buf.Truncate(start) + buf.WriteByte(' ') + buf.Write(operand) + } case Prior: buf.WriteString("PRIOR ") writeExpr(buf, u.Expr) diff --git a/pkg/sql/ast/sql_unary_minus_test.go b/pkg/sql/ast/sql_unary_minus_test.go new file mode 100644 index 00000000..ef2157ea --- /dev/null +++ b/pkg/sql/ast/sql_unary_minus_test.go @@ -0,0 +1,69 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ast + +import ( + "strings" + "testing" +) + +// TestUnaryMinusSQL_NoLineComment checks that a unary minus whose operand +// itself renders with a leading '-' is separated from it: "--" starts a SQL +// line comment and would silently comment out the rest of the line. +func TestUnaryMinusSQL_NoLineComment(t *testing.T) { + one := func() Expression { return &LiteralValue{Value: "1", Type: "INTEGER"} } + tests := []struct { + name string + expr Expression + want string + }{ + { + name: "minus minus literal", + expr: &UnaryExpression{Operator: Minus, Expr: &UnaryExpression{Operator: Minus, Expr: one()}}, + want: "- -1", + }, + { + name: "minus negative literal", + expr: &UnaryExpression{Operator: Minus, Expr: &LiteralValue{Value: "-1", Type: "INTEGER"}}, + want: "- -1", + }, + { + name: "triple minus", + expr: &UnaryExpression{Operator: Minus, Expr: &UnaryExpression{Operator: Minus, Expr: &UnaryExpression{Operator: Minus, Expr: one()}}}, + want: "- - -1", + }, + { + name: "minus identifier", + expr: &UnaryExpression{Operator: Minus, Expr: &Identifier{Name: "x"}}, + want: "-x", + }, + { + name: "plus minus", + expr: &UnaryExpression{Operator: Plus, Expr: &UnaryExpression{Operator: Minus, Expr: one()}}, + want: "+-1", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.expr.(interface{ SQL() string }).SQL() + if got != tc.want { + t.Fatalf("SQL() = %q, want %q", got, tc.want) + } + if strings.Contains(got, "--") { + t.Fatalf("SQL() = %q contains a line comment marker", got) + } + }) + } +}