From 880b191fbb7094bd9b517e1d2d4f860d2ee8f5ed Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Wed, 28 Oct 2026 10:59:41 +0530 Subject: [PATCH 01/10] fix(parser): end VACUUM, ANALYZE and LISTEN at their grammar PostgreSQL, Redshift and SQLite VACUUM / ANALYZE and PostgreSQL LISTEN, UNLISTEN and NOTIFY were kept verbatim, so trailing same-line tokens (VACUUM t junk) were swallowed into the statement text. They now follow the GRANT / COPY pattern: a small per-dialect grammar finds the end and utilityTailError rejects anything after it. Generic mode accepts the union of the dialect forms; MySQL / MariaDB / Oracle ANALYZE TABLE keeps its verbatim path. Adds regression tests for the already-fixed CREATE TRIGGER / PREPARE / GRANT / COPY boundaries and for the splitter's MySQL backslash escapes and PostgreSQL CREATE RULE ... DO (a; b) command lists. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 5 + pkg/gosqlx/splitter_test.go | 31 +++ pkg/sql/parser/utility_maintenance.go | 249 +++++++++++++++++++ pkg/sql/parser/utility_maintenance_test.go | 159 ++++++++++++ pkg/sql/parser/utility_statement_end_test.go | 16 ++ pkg/sql/parser/utility_stmt.go | 13 +- 6 files changed, 471 insertions(+), 2 deletions(-) create mode 100644 pkg/sql/parser/utility_maintenance.go create mode 100644 pkg/sql/parser/utility_maintenance_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2935c0517..9361adad6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 in dialects without them. ### Fixed +- PostgreSQL / Redshift / SQLite `VACUUM` and `ANALYZE`, and PostgreSQL `LISTEN`, + `UNLISTEN` and `NOTIFY`, now end where their grammar ends: trailing same-line + tokens (`VACUUM t junk`, `LISTEN c extra`) are a parse error instead of being + folded into the statement text. MySQL / MariaDB / Oracle `ANALYZE TABLE` keeps + its as-written handling. - Watch now respects `--dialect`, re-validates changes and formats files in place with the CLI safety checks; MCP parsing accepts a dialect and documents comment preservation. - Correct transpile dialect help, avoid duplicate lint syntax errors, and rewrite diff --git a/pkg/gosqlx/splitter_test.go b/pkg/gosqlx/splitter_test.go index 9e4f81fed..a54bac34c 100644 --- a/pkg/gosqlx/splitter_test.go +++ b/pkg/gosqlx/splitter_test.go @@ -288,6 +288,37 @@ func TestSplitStatements_NilOnEmpty(t *testing.T) { } } +func TestSplitStatements_MySQLBackslashBoundaries(t *testing.T) { + for _, dialect := range []string{"mysql", "mariadb"} { + for _, tc := range []struct { + name string + first string + }{ + {"escaped quote", `SELECT 'it\'s; here'`}, + {"escaped backslash", `SELECT 'a\\'`}, + } { + t.Run(dialect+"/"+tc.name, func(t *testing.T) { + want := []string{tc.first, " SELECT 2"} + if got := SplitStatements(tc.first+"; SELECT 2", dialect); !reflect.DeepEqual(got, want) { + t.Fatalf("segments = %q, want %q", got, want) + } + }) + } + } +} + +func TestSplitStatements_PostgreSQLRuleCommandList(t *testing.T) { + for _, action := range []string{"DO", "DO ALSO", "DO INSTEAD"} { + t.Run(action, func(t *testing.T) { + first := "CREATE RULE r AS ON INSERT TO t " + action + " (INSERT INTO a VALUES (1); INSERT INTO b VALUES (2))" + want := []string{first, " SELECT 1"} + if got := SplitStatements(first+"; SELECT 1", "postgresql"); !reflect.DeepEqual(got, want) { + t.Fatalf("segments = %q, want %q", got, want) + } + }) + } +} + func TestSplitStatements_TilesSource(t *testing.T) { root := filepath.Join("..", "..", "testdata") files := 0 diff --git a/pkg/sql/parser/utility_maintenance.go b/pkg/sql/parser/utility_maintenance.go new file mode 100644 index 000000000..e5c7756bd --- /dev/null +++ b/pkg/sql/parser/utility_maintenance.go @@ -0,0 +1,249 @@ +// 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 parser + +import "github.com/ajitpratap0/GoSQLX/pkg/models" + +// maintenanceEnd selects a complete dialect grammar. Without a dialect it +// accepts the union, choosing the longest successful form rather than mixing +// options from different dialects. The caller checks the remaining tokens. +func (p *Parser) maintenanceEnd(start int, postgres, redshift, sqlite func(*txnCursor) error) (int, error) { + mask := p.alterMask() + best := -1 + var firstErr error + for _, form := range []struct { + dialect alterDialects + parse func(*txnCursor) error + }{{dialectPostgres, postgres}, {dialectRedshift, redshift}, {dialectSQLite, sqlite}} { + if mask&form.dialect == 0 { + continue + } + c := txnCursor{p: p, i: start + 1, mask: mask} + if err := form.parse(&c); err != nil { + if firstErr == nil { + firstErr = err + } + } else if c.i > best { + best = c.i + } + } + if best >= 0 { + return best, nil + } + return 0, firstErr +} + +func (p *Parser) vacuumEnd(start int) (int, error) { + return p.maintenanceEnd(start, (*txnCursor).postgresVacuum, (*txnCursor).redshiftVacuum, (*txnCursor).sqliteVacuum) +} + +func (p *Parser) analyzeEnd(start int) (int, error) { + return p.maintenanceEnd(start, (*txnCursor).postgresAnalyze, (*txnCursor).redshiftAnalyze, (*txnCursor).sqliteAnalyze) +} + +// maintenanceNameAt excludes statement starters so a bare maintenance command +// does not swallow a following statement's first word as its optional table. +func (c *txnCursor) maintenanceNameAt() bool { + return c.p.isNamePartAt(c.i) && !implicitStartWords[c.word()] +} + +// maintenanceGroup consumes balanced options or column lists. Their contents +// are kept as written, like the parenthesized COPY options. +func (c *txnCursor) maintenanceGroup() error { + if c.typeAt(c.i) != models.TokenTypeLParen { + return nil + } + end := c.p.groupEnd(c.i) + if end < 0 { + return c.fail(")") + } + c.i = end + return nil +} + +func (c *txnCursor) maintenanceTable(columns bool) error { + if !c.maintenanceNameAt() { + return c.fail("table name") + } + c.i = c.p.nameEnd(c.i) + if columns { + return c.maintenanceGroup() + } + return nil +} + +// postgresTables consumes [ONLY] qualified_name [(columns)] [, ...]. +func (c *txnCursor) postgresTables() error { + if c.word() != "ONLY" && !c.maintenanceNameAt() { + return nil + } + for { + c.accept("ONLY") + if err := c.maintenanceTable(true); err != nil { + return err + } + if c.typeAt(c.i) != models.TokenTypeComma { + return nil + } + c.i++ + } +} + +func (c *txnCursor) postgresVacuum() error { + if c.typeAt(c.i) == models.TokenTypeLParen { + if err := c.maintenanceGroup(); err != nil { + return err + } + } else { + // Legacy options are accepted in any order, once each. + var seen uint8 + for { + var bit uint8 + switch c.word() { + case "FULL": + bit = 1 + case "FREEZE": + bit = 2 + case "VERBOSE": + bit = 4 + case "ANALYZE": + bit = 8 + } + if bit == 0 || seen&bit != 0 { + break + } + seen |= bit + c.i++ + } + } + return c.postgresTables() +} + +func (c *txnCursor) postgresAnalyze() error { + if c.typeAt(c.i) == models.TokenTypeLParen { + if err := c.maintenanceGroup(); err != nil { + return err + } + } else { + c.accept("VERBOSE") + } + return c.postgresTables() +} + +func (c *txnCursor) redshiftVacuum() error { + switch c.word() { + case "SORT", "DELETE": + c.i++ + if err := c.expect("ONLY"); err != nil { + return err + } + case "FULL", "REINDEX", "RECLUSTER": + c.i++ + } + if c.word() != "TO" && c.word() != "BOOST" && c.maintenanceNameAt() { + if err := c.maintenanceTable(false); err != nil { + return err + } + } + if c.accept("TO") { + if err := c.maintenanceNumber(); err != nil { + return err + } + if err := c.expect("PERCENT"); err != nil { + return err + } + } + c.accept("BOOST") + return nil +} + +func (c *txnCursor) redshiftAnalyze() error { + compression := c.accept("COMPRESSION") + if !compression { + c.accept("VERBOSE") + } + if c.word() != "PREDICATE" && c.word() != "ALL" && c.word() != "COMPROWS" && c.maintenanceNameAt() { + if err := c.maintenanceTable(true); err != nil { + return err + } + } + if compression { + if c.accept("COMPROWS") { + return c.maintenanceNumber() + } + } else if c.accept("PREDICATE") || c.accept("ALL") { + return c.expect("COLUMNS") + } + return nil +} + +func (c *txnCursor) maintenanceNumber() error { + if c.typeAt(c.i) != models.TokenTypeNumber { + return c.fail("number") + } + c.i++ + return nil +} + +func (c *txnCursor) sqliteVacuum() error { + if c.word() != "INTO" && c.maintenanceNameAt() { + c.i++ // SQLite accepts a single schema identifier here. + } + if c.accept("INTO") { + switch { + case c.copyString(): + case c.typeAt(c.i) == models.TokenTypePlaceholder: + c.i++ + case c.maintenanceNameAt(): + c.i = c.p.nameEnd(c.i) + default: + return c.fail("string, placeholder or name") + } + } + return nil +} + +func (c *txnCursor) sqliteAnalyze() error { + if c.maintenanceNameAt() { + end := c.p.nameEnd(c.i) + // SQLite permits only [schema.]object, unlike PostgreSQL names. + if end > c.i+3 { + end = c.i + 3 + } + c.i = end + } + return nil +} + +// listenEnd handles LISTEN, UNLISTEN and NOTIFY. Channels are single names; +// NOTIFY's only optional argument is a string payload. +func (p *Parser) listenEnd(start int) (int, error) { + c := txnCursor{p: p, i: start + 1, mask: p.alterMask()} + kind := p.utilityWord(start) + if kind == "UNLISTEN" && (c.typeAt(c.i) == models.TokenTypeMul || c.typeAt(c.i) == models.TokenTypeAsterisk) { + return c.i + 1, nil + } + if !c.maintenanceNameAt() { + return 0, c.fail("channel name") + } + c.i++ + if kind == "NOTIFY" && c.typeAt(c.i) == models.TokenTypeComma { + c.i++ + if !c.copyString() { + return 0, c.fail("string payload") + } + } + return c.i, nil +} diff --git a/pkg/sql/parser/utility_maintenance_test.go b/pkg/sql/parser/utility_maintenance_test.go new file mode 100644 index 000000000..0b1f52d58 --- /dev/null +++ b/pkg/sql/parser/utility_maintenance_test.go @@ -0,0 +1,159 @@ +// 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 parser + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestParser_MaintenanceBoundaries(t *testing.T) { + pg, rs, lite := keywords.DialectPostgreSQL, keywords.DialectRedshift, keywords.DialectSQLite + tests := []struct { + dialect keywords.SQLDialect + sql string + }{ + {pg, "VACUUM t junk words here"}, {pg, "VACUUM (VERBOSE) t junk"}, + {pg, "VACUUM FULL t garbage"}, {pg, "ANALYZE t foo bar"}, + {pg, "ANALYZE VERBOSE t (a) extra"}, {pg, "LISTEN chan extra stuff"}, + {pg, "LISTEN a, b"}, {pg, "UNLISTEN chan junk"}, {pg, "NOTIFY chan, 'p' junk"}, + {pg, "VACUUM t junk; SELECT 1"}, {rs, "VACUUM FULL t TO 75 PERCENT junk"}, + {lite, "VACUUM junk more"}, {lite, "VACUUM main INTO 'f.db' junk"}, + {lite, "ANALYZE main.t junk"}, {lite, "ANALYZE main.t.extra"}, {"", "VACUUM t junk words"}, + {"", "VACUUM (VERBOSE) t BOOST"}, {"", "VACUUM FULL t INTO 'f.db'"}, + } + for _, tc := range tests { + t.Run(string(tc.dialect)+"/"+tc.sql, func(t *testing.T) { + tree, err := ParseWithDialect(tc.sql, tc.dialect) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err == nil || !strings.Contains(err.Error(), "semicolon or end of statement") { + t.Fatalf("got %v, want trailing-token error", err) + } + }) + } +} + +func TestParser_MaintenanceForms(t *testing.T) { + tests := []struct { + dialect keywords.SQLDialect + forms []string + }{ + {keywords.DialectPostgreSQL, []string{ + "VACUUM", "VACUUM FULL t", "VACUUM FULL FREEZE VERBOSE ANALYZE t", "VACUUM ANALYZE t", + "VACUUM (VERBOSE, ANALYZE) users", "VACUUM (PARALLEL 4, INDEX_CLEANUP off) s.t (a, b), u", + "VACUUM ONLY t", "VACUUM t, u", "VACUUM t (a, b)", + "ANALYZE", "ANALYZE users", "ANALYZE VERBOSE t (a)", "ANALYZE (SKIP_LOCKED) t", + "LISTEN channel", `LISTEN "My Chan"`, "UNLISTEN *", "UNLISTEN c", "NOTIFY channel", "NOTIFY channel, 'hello'", + `NOTIFY channel, E'hello'`, "NOTIFY channel, $$hello$$", "VACUUM t; SELECT 1", "LISTEN c\nCOMMIT", + }}, + {keywords.DialectRedshift, []string{ + "VACUUM", "VACUUM FULL t", "VACUUM SORT ONLY t", "VACUUM DELETE ONLY t TO 90 PERCENT", + "VACUUM REINDEX t", "VACUUM RECLUSTER t", "VACUUM t TO 75 PERCENT BOOST", + "ANALYZE", "ANALYZE t PREDICATE COLUMNS", "ANALYZE VERBOSE t (a, b)", + "ANALYZE ALL COLUMNS", "ANALYZE COMPRESSION", "ANALYZE COMPRESSION t COMPROWS 1000", + }}, + {keywords.DialectSQLite, []string{ + "VACUUM", "VACUUM main", "VACUUM INTO 'backup.db'", "VACUUM main INTO 'f.db'", + "VACUUM main INTO ?", "VACUUM INTO backup", "ANALYZE", "ANALYZE main", "ANALYZE main.t", + }}, + {keywords.DialectMySQL, []string{"ANALYZE TABLE t, u", "ANALYZE NO_WRITE_TO_BINLOG TABLE t UPDATE HISTOGRAM ON c WITH 10 BUCKETS"}}, + {keywords.DialectOracle, []string{"ANALYZE TABLE t COMPUTE STATISTICS", "ANALYZE TABLE t DELETE STATISTICS"}}, + {"", []string{"VACUUM (VERBOSE) t", "VACUUM SORT ONLY t", "VACUUM main INTO 'f.db'", "ANALYZE t, u", "ANALYZE COMPRESSION t COMPROWS 1000", "ANALYZE TABLE t", "LISTEN c"}}, + } + for _, tc := range tests { + for _, sql := range tc.forms { + t.Run(string(tc.dialect)+"/"+sql, func(t *testing.T) { + tree, err := ParseWithDialect(sql, tc.dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + count := 1 + if strings.Contains(sql, ";") || strings.Contains(sql, "\n") { + count = 2 + } + if len(tree.Statements) != count { + t.Fatalf("got %d statements, want %d", len(tree.Statements), count) + } + stmt, ok := tree.Statements[0].(*ast.UtilityStatement) + if !ok { + t.Fatalf("got %T, want UtilityStatement", tree.Statements[0]) + } + if want := strings.Fields(sql)[0]; stmt.Kind != want { + t.Errorf("Kind = %q, want %q", stmt.Kind, want) + } + if stmt.Text == "" { + t.Error("empty statement text") + } + }) + } + } +} + +func TestParser_MaintenanceText(t *testing.T) { + for _, sql := range []string{`vacuum (VERBOSE) "My Table" (a, b)`, `listen "My Chan"`, `notify c, 'hello'`} { + t.Run(sql, func(t *testing.T) { + tree, err := ParseWithDialect(sql+"; SELECT 1", keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + stmt, ok := tree.Statements[0].(*ast.UtilityStatement) + if !ok { + t.Fatalf("got %T, want UtilityStatement", tree.Statements[0]) + } + if stmt.Text != sql { + t.Errorf("Text = %q, want %q", stmt.Text, sql) + } + }) + } +} + +func TestParser_MaintenanceIncomplete(t *testing.T) { + tests := []struct { + dialect keywords.SQLDialect + sql string + }{ + {keywords.DialectPostgreSQL, "VACUUM (VERBOSE"}, + {keywords.DialectPostgreSQL, "VACUUM t (a"}, + {keywords.DialectPostgreSQL, "VACUUM ONLY"}, + {keywords.DialectPostgreSQL, "ANALYZE t,"}, + {keywords.DialectPostgreSQL, "LISTEN"}, + {keywords.DialectPostgreSQL, "NOTIFY c,"}, + {keywords.DialectPostgreSQL, "NOTIFY c, 1"}, + {keywords.DialectRedshift, "VACUUM SORT"}, + {keywords.DialectRedshift, "VACUUM t TO PERCENT"}, + {keywords.DialectRedshift, "VACUUM t TO 75"}, + {keywords.DialectRedshift, "ANALYZE t PREDICATE"}, + {keywords.DialectRedshift, "ANALYZE COMPRESSION t COMPROWS"}, + {keywords.DialectSQLite, "VACUUM INTO"}, + } + for _, tc := range tests { + t.Run(string(tc.dialect)+"/"+tc.sql, func(t *testing.T) { + tree, err := ParseWithDialect(tc.sql, tc.dialect) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err == nil || !strings.Contains(err.Error(), "E2002 at line 1, column") { + t.Fatalf("got %v, want positioned E2002", err) + } + }) + } +} diff --git a/pkg/sql/parser/utility_statement_end_test.go b/pkg/sql/parser/utility_statement_end_test.go index 66e83606c..c165e14d2 100644 --- a/pkg/sql/parser/utility_statement_end_test.go +++ b/pkg/sql/parser/utility_statement_end_test.go @@ -152,6 +152,22 @@ func TestParser_NestedStatementBodyEnds(t *testing.T) { }) } +func TestParser_UtilitySelectBoundaryRegressions(t *testing.T) { + pg := keywords.DialectPostgreSQL + runShapeCases(t, []shapeCase{ + {"pg trigger then select", pg, + "CREATE TRIGGER trg AFTER INSERT ON t FOR EACH ROW EXECUTE FUNCTION f()\nSELECT 1", + "error: end of statement"}, + {"pg trigger separated select", pg, + "CREATE TRIGGER trg AFTER INSERT ON t FOR EACH ROW EXECUTE FUNCTION f(); SELECT 1", + "[Utility(CREATE TRIGGER) Select]"}, + {"prepare same line select", pg, "PREPARE p AS SELECT 1 SELECT 2", "error: end of statement"}, + {"prepare separated select", pg, "PREPARE p AS SELECT 1; SELECT 2", "[Utility(PREPARE) Select]"}, + {"pg grant then select", pg, "GRANT SELECT ON t TO bob SELECT 1", "error: semicolon or end of statement"}, + {"pg copy then select", pg, "COPY t TO STDOUT SELECT 1", "error: semicolon or end of statement"}, + }) +} + // GRANT, REVOKE, COPY, CALL and the other utility statements with a closed // grammar end where it ends, so a statement written after them on the same // line without a ';' is an error rather than part of their text. diff --git a/pkg/sql/parser/utility_stmt.go b/pkg/sql/parser/utility_stmt.go index 3cbda0ddc..f69346305 100644 --- a/pkg/sql/parser/utility_stmt.go +++ b/pkg/sql/parser/utility_stmt.go @@ -130,6 +130,11 @@ func (p *Parser) parseUtilityStatement() (stmt ast.Statement, ok bool, err error s, err := p.parseDescribeStatement("ANALYZE") return s, true, err } + mask := p.alterMask() + if mask == dialectAll && next != "TABLE" && next != "LOCAL" && next != "NO_WRITE_TO_BINLOG" || + mask != dialectAll && mask&(txnPostgres|dialectSQLite) != 0 { + return p.parseGrammarStatement(word, p.analyzeEnd) + } return p.parseVerbatimStatement(word) case "COMMENT": if next != "ON" { @@ -200,8 +205,12 @@ func (p *Parser) parseUtilityStatement() (stmt ast.Statement, ok bool, err error return nil, false, nil } return p.parseVerbatimArgs(word) - case "VACUUM", "LOCK", "UNLOCK", "KILL", "FLUSH", "OPTIMIZE", - "CHECK", "REPAIR", "CHECKSUM", "LISTEN", "NOTIFY", "UNLISTEN", "PREPARE", + case "VACUUM": + return p.parseGrammarStatement(word, p.vacuumEnd) + case "LISTEN", "UNLISTEN", "NOTIFY": + return p.parseGrammarStatement(word, p.listenEnd) + case "LOCK", "UNLOCK", "KILL", "FLUSH", "OPTIMIZE", + "CHECK", "REPAIR", "CHECKSUM", "PREPARE", "EXECUTE", "DEALLOCATE", "DECLARE", "RENAME", "REINDEX", "CLUSTER", "CHECKPOINT", "DISCARD", "RESET", "XA", "BACKUP", "FLASHBACK", "PURGE", "UNDROP", "EXCHANGE", "SYSTEM", "ATTACH", "DETACH", "HANDLER", From cff610da707202f96e13a1a54142046a55050c32 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Wed, 28 Oct 2026 11:01:23 +0530 Subject: [PATCH 02/10] fix(security): walk the full AST in Scanner.Scan for ScanSQL parity Scan(tree) only inspected top-level SELECT WHERE/HAVING/columns/FROM, INSERT VALUES rows, UPDATE/DELETE WHERE and set-operation leaves, so time-based, out-of-band and tautology patterns inside CTEs, subqueries, derived tables, CASE/CAST/IN/BETWEEN, ORDER/GROUP BY, JOIN ON, window specs, FILTER, LIMIT, LATERAL/APPLY, PREWHERE, QUALIFY, CONNECT BY, RETURNING, ON CONFLICT, INSERT ... SELECT, MERGE, CREATE TABLE AS, CREATE [MATERIALIZED] VIEW and EXPLAIN went undetected while ScanSQL caught them. Scan now uses ast.Inspect: every FunctionCall and table function is checked once, tautologies are checked in every condition position, UNION checks run on each outermost set operation (including those nested in CTEs and subqueries), and text-bodied nodes keep the text path. JOIN Left references are skipped to avoid double counting, and qualified identifiers such as a.id = b.id are no longer reported as tautologies. T-SQL IF ... WAITFOR and standalone VALUES were already covered on main (#871) and gain regression tests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 8 + pkg/sql/security/ast_coverage_test.go | 253 ++++++++++++++++++++++ pkg/sql/security/scanner.go | 294 +++++++++----------------- 3 files changed, 356 insertions(+), 199 deletions(-) create mode 100644 pkg/sql/security/ast_coverage_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2935c0517..9d1f96c10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 in dialects without them. ### Fixed +- **Security scanner**: `Scanner.Scan(tree)` now walks the whole AST, so time-based, + out-of-band and tautology patterns are detected inside CTEs, derived tables and + subqueries, CASE/CAST/IN/BETWEEN/ANY, ORDER BY/GROUP BY, JOIN ON, window specs, + FILTER, LIMIT, LATERAL/APPLY, PREWHERE, QUALIFY, CONNECT BY, RETURNING, ON CONFLICT, + INSERT ... SELECT, MERGE, CREATE TABLE AS, CREATE [MATERIALIZED] VIEW and EXPLAIN, + matching `ScanSQL`. UNION injection checks also run on set operations nested in CTEs + and subqueries. Qualified column comparisons such as `a.id = b.id` are no longer + reported as tautologies. - Watch now respects `--dialect`, re-validates changes and formats files in place with the CLI safety checks; MCP parsing accepts a dialect and documents comment preservation. - Correct transpile dialect help, avoid duplicate lint syntax errors, and rewrite diff --git a/pkg/sql/security/ast_coverage_test.go b/pkg/sql/security/ast_coverage_test.go new file mode 100644 index 000000000..46f14799d --- /dev/null +++ b/pkg/sql/security/ast_coverage_test.go @@ -0,0 +1,253 @@ +// 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 security + +import ( + "reflect" + "sync" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" + "github.com/ajitpratap0/GoSQLX/pkg/sql/parser" +) + +type astCoverageCase struct { + name string + sql string + dialect keywords.SQLDialect + patterns []PatternType + exact map[PatternType]int +} + +func astCoverageCases() []astCoverageCase { + const pg = keywords.DialectPostgreSQL + tb := []PatternType{PatternTimeBased} + taut := []PatternType{PatternTautology} + oob := []PatternType{PatternOutOfBand} + oneTB := map[PatternType]int{PatternTimeBased: 1} + oneOOB := map[PatternType]int{PatternOutOfBand: 1} + return []astCoverageCase{ + {"01 CTE function", "WITH c AS (SELECT pg_sleep(5)) SELECT * FROM c", pg, tb, oneTB}, + {"02 CTE condition", "WITH c AS (SELECT * FROM t WHERE 1=1 OR 2=2) SELECT * FROM c", pg, taut, nil}, + {"03 derived table", "SELECT * FROM (SELECT pg_sleep(5)) s", pg, tb, oneTB}, + {"04 IN subquery", "SELECT * FROM t WHERE id IN (SELECT pg_sleep(5))", pg, tb, oneTB}, + {"05 EXISTS", "SELECT * FROM t WHERE EXISTS (SELECT pg_sleep(5))", pg, tb, nil}, + {"06 scalar subquery", "SELECT * FROM t WHERE x = (SELECT pg_sleep(5))", pg, tb, oneTB}, + {"07 CASE", "SELECT CASE WHEN 1=1 THEN pg_sleep(5) END FROM t", pg, []PatternType{PatternTimeBased, PatternTautology}, oneTB}, + {"08 CAST", "SELECT CAST(pg_sleep(5) AS text) FROM t", pg, tb, nil}, + {"09 BETWEEN", "SELECT * FROM t WHERE x BETWEEN pg_sleep(1) AND 2", pg, tb, nil}, + {"10 IN list", "SELECT * FROM t WHERE x IN (pg_sleep(1), 2)", pg, tb, nil}, + {"11 ORDER BY", "SELECT * FROM t ORDER BY pg_sleep(5)", pg, tb, nil}, + {"12 GROUP BY", "SELECT * FROM t GROUP BY pg_sleep(5)", pg, tb, nil}, + {"13 JOIN condition", "SELECT * FROM a JOIN b ON a.id = b.id OR 1=1", pg, taut, nil}, + {"14 JOIN function", "SELECT * FROM a JOIN b ON pg_sleep(5) IS NULL", pg, tb, oneTB}, + {"15 window", "SELECT count(*) OVER (ORDER BY pg_sleep(5)) FROM t", pg, tb, nil}, + {"16 INSERT SELECT", "INSERT INTO t SELECT pg_sleep(5)", pg, tb, nil}, + {"17 ON CONFLICT", "INSERT INTO t VALUES (1) ON CONFLICT (id) DO UPDATE SET x = pg_sleep(5)", pg, tb, nil}, + {"18 INSERT RETURNING", "INSERT INTO t VALUES (1) RETURNING pg_sleep(5)", pg, tb, nil}, + {"19 UPDATE RETURNING", "UPDATE t SET x = 1 RETURNING pg_sleep(5)", pg, tb, nil}, + {"20 DELETE subquery", "DELETE FROM t WHERE id IN (SELECT id FROM u WHERE 1=1 OR 2=2)", pg, taut, nil}, + {"21 MERGE UPDATE", "MERGE INTO t USING s ON t.id = s.id OR 1=1 WHEN MATCHED THEN UPDATE SET x = pg_sleep(5)", pg, []PatternType{PatternTautology, PatternTimeBased}, nil}, + {"22 MERGE INSERT", "MERGE INTO t USING s ON t.id = s.id WHEN NOT MATCHED THEN INSERT (a) VALUES (pg_sleep(5))", pg, tb, nil}, + {"23 CREATE TABLE AS", "CREATE TABLE x AS SELECT pg_sleep(5)", pg, tb, nil}, + {"24 CREATE VIEW", "CREATE VIEW v AS SELECT pg_sleep(5)", pg, tb, nil}, + {"25 CREATE MATERIALIZED VIEW", "CREATE MATERIALIZED VIEW v AS SELECT pg_sleep(5)", pg, tb, nil}, + {"26 EXPLAIN", "EXPLAIN SELECT pg_sleep(5)", pg, tb, nil}, + {"27 UNION derived table", "SELECT 1 UNION SELECT * FROM (SELECT pg_sleep(5)) s", pg, tb, oneTB}, + {"28 ANY", "SELECT * FROM t WHERE x = ANY (SELECT pg_sleep(5))", pg, tb, nil}, + {"29 ARRAY", "SELECT ARRAY[pg_sleep(5)]", pg, tb, nil}, + {"30 tuple", "SELECT (pg_sleep(5), 1)", pg, tb, nil}, + {"31 FILTER", "SELECT coalesce(x, 1) FILTER (WHERE pg_sleep(5) IS NULL) FROM t", pg, tb, nil}, + {"32 LIMIT", "SELECT * FROM t LIMIT (SELECT pg_sleep(5))", pg, tb, nil}, + {"33 LATERAL", "SELECT * FROM t, LATERAL (SELECT pg_sleep(5)) s", pg, tb, nil}, + {"34 cast operator", "SELECT x::text FROM t WHERE pg_sleep(5)::text = ''", pg, tb, nil}, + {"35 EXTRACT", "SELECT EXTRACT(YEAR FROM pg_sleep(5))", pg, tb, nil}, + {"36 nested VALUES", "SELECT * FROM (VALUES (pg_sleep(5))) v", pg, tb, oneTB}, + {"37 CTE UNION", "WITH c AS (SELECT id FROM t UNION SELECT table_name FROM information_schema.tables) SELECT * FROM c", pg, []PatternType{PatternUnionBased}, nil}, + {"38 WHERE function", "SELECT * FROM t WHERE pg_sleep(5) IS NULL", pg, tb, oneTB}, + {"39a function", "SELECT pg_sleep(5)", pg, tb, oneTB}, + {"39b qualified function", "SELECT pg_catalog.pg_sleep(5)", pg, tb, oneTB}, + {"40 CONNECT BY", "SELECT * FROM t START WITH id = 1 CONNECT BY PRIOR id = pid AND DBMS_LOCK.SLEEP(5) = 1", keywords.DialectOracle, tb, nil}, + {"41 PREWHERE", "SELECT * FROM t PREWHERE sleep(3) = 0", keywords.DialectClickHouse, tb, nil}, + {"42 table function", "SELECT * FROM url('http://x/a.csv', CSV, 'a String')", keywords.DialectClickHouse, oob, oneOOB}, + {"43 JOIN table function", "SELECT * FROM t JOIN url('http://x/a.csv', CSV, 'a String') u ON 1 = 1", keywords.DialectClickHouse, oob, oneOOB}, + {"44 APPLY", "SELECT TOP (1) * FROM t CROSS APPLY (SELECT xp_cmdshell('dir')) s", keywords.DialectSQLServer, oob, nil}, + {"45 IF WAITFOR", "IF 1=1 WAITFOR DELAY '0:0:5'", keywords.DialectSQLServer, []PatternType{PatternTimeBased, PatternTautology}, nil}, + {"46 IF subquery WAITFOR", "IF (SELECT COUNT(*) FROM users) > 0 WAITFOR DELAY '0:0:5'", keywords.DialectSQLServer, tb, nil}, + {"47 IF BEGIN WAITFOR", "IF 1=1 BEGIN WAITFOR DELAY '0:0:5' END", keywords.DialectSQLServer, tb, nil}, + {"48 AND function", "SELECT * FROM t WHERE id = 1 AND SLEEP(5)", keywords.DialectMySQL, tb, oneTB}, + {"49 LIKE function", "SELECT * FROM t WHERE x LIKE SLEEP(5)", keywords.DialectMySQL, tb, oneTB}, + {"50 QUALIFY", "SELECT * FROM t QUALIFY ROW_NUMBER() OVER (ORDER BY pg_sleep(5)) = 1", keywords.DialectSnowflake, tb, nil}, + {"FILTER condition", "SELECT count(*) FILTER (WHERE 1=1) FROM t", pg, taut, nil}, + {"ON CONFLICT target condition", "INSERT INTO t VALUES (1) ON CONFLICT (id) WHERE 1=1 DO NOTHING", pg, taut, nil}, + {"ON CONFLICT action condition", "INSERT INTO t VALUES (1) ON CONFLICT (id) DO UPDATE SET x = 1 WHERE 1=1", pg, taut, nil}, + {"QUALIFY condition", "SELECT * FROM t QUALIFY 1=1", keywords.DialectSnowflake, taut, nil}, + {"PREWHERE condition", "SELECT * FROM t PREWHERE 1=1", keywords.DialectClickHouse, taut, nil}, + {"START WITH condition", "SELECT * FROM t START WITH 1=1 CONNECT BY PRIOR id = pid", keywords.DialectOracle, taut, nil}, + {"CONNECT BY condition", "SELECT * FROM t START WITH id = 1 CONNECT BY 1=1", keywords.DialectOracle, taut, nil}, + {"MERGE WHEN condition", "MERGE INTO t USING s ON t.id = s.other_id WHEN MATCHED AND 1=1 THEN UPDATE SET x = s.x", pg, taut, nil}, + {"JOIN MATCH_CONDITION", "SELECT * FROM t1 ASOF JOIN t2 MATCH_CONDITION (1=1) ON t1.sym = t2.other_sym", keywords.DialectSnowflake, taut, nil}, + {"boolean parentheses and NOT", "SELECT * FROM t WHERE NOT (x = 1 OR (1=1))", pg, taut, nil}, + {"nested dangerous function", "WITH c AS (SELECT CASE WHEN x > 1 THEN xp_cmdshell('dir') END FROM t) SELECT * FROM c", keywords.DialectSQLServer, oob, oneOOB}, + {"standalone WAITFOR", "WAITFOR DELAY '0:0:5'", keywords.DialectSQLServer, tb, nil}, + {"EXEC utility", "EXEC xp_cmdshell 'dir'", keywords.DialectSQLServer, oob, nil}, + {"standalone VALUES", "VALUES (pg_sleep(5))", pg, tb, oneTB}, + {"standalone VALUES CASE", "VALUES (CASE WHEN 1=1 THEN pg_sleep(5) END)", pg, []PatternType{PatternTimeBased, PatternTautology}, oneTB}, + } +} + +func astCoveragePatternCount(result *ScanResult, pattern PatternType) int { + count := 0 + for _, finding := range result.Findings { + if finding.Pattern == pattern { + count++ + } + } + return count +} + +func TestScan_ASTCoverage(t *testing.T) { + scanner := NewScanner() + for _, tt := range astCoverageCases() { + t.Run(tt.name, func(t *testing.T) { + tree, err := parser.ParseWithDialect(tt.sql, tt.dialect) + if err != nil { + t.Fatalf("parse %q: %v", tt.sql, err) + } + defer ast.ReleaseAST(tree) + result := scanner.Scan(tree) + for _, pattern := range tt.patterns { + if astCoveragePatternCount(result, pattern) == 0 { + t.Errorf("Scan missing %s: %+v", pattern, result.Findings) + } + } + for pattern, want := range tt.exact { + if got := astCoveragePatternCount(result, pattern); got != want { + t.Errorf("Scan %s count = %d, want %d; findings: %+v", pattern, got, want, result.Findings) + } + } + textResult := scanner.ScanSQL(tt.sql) + for _, pattern := range []PatternType{PatternTimeBased, PatternOutOfBand, PatternTautology} { + if hasPattern(textResult, pattern) && !hasPattern(result, pattern) { + t.Errorf("ScanSQL reports %s but Scan does not; text: %+v; AST: %+v", pattern, textResult.Findings, result.Findings) + } + } + }) + } +} + +func TestScan_ASTCoverageClean(t *testing.T) { + tests := []string{ + "SELECT 1=1 FROM t ORDER BY 2=2", + "SELECT f(1=1) FROM t", + "WITH c AS (SELECT id FROM t WHERE id = 1) SELECT * FROM c", + "MERGE INTO t USING s ON t.id = s.id WHEN MATCHED THEN UPDATE SET x = s.x WHEN NOT MATCHED THEN INSERT (id, x) VALUES (s.id, s.x)", + "SELECT CASE WHEN a > 1 THEN 'x' ELSE 'y' END FROM t ORDER BY a", + "SELECT * FROM a JOIN b ON a.id = b.id WHERE b.x IN (SELECT x FROM c WHERE c.y = 2)", + "CREATE VIEW v AS SELECT id, name FROM users WHERE active = true", + } + for _, sql := range tests { + t.Run(sql, func(t *testing.T) { + tree, err := parser.ParseWithDialect(sql, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + if result := NewScanner().Scan(tree); !result.IsClean() { + t.Errorf("expected clean Scan, got %+v", result.Findings) + } + }) + } +} + +func TestScan_ASTCoverageUnion(t *testing.T) { + scanner := NewScanner() + const unionSQL = "SELECT id FROM t UNION SELECT table_name FROM information_schema.tables" + baselineTree, err := parser.ParseWithDialect(unionSQL, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(baselineTree) + baseline := scanner.Scan(baselineTree) + if countUnionSystemTableFindings(baseline) == 0 { + t.Fatalf("top-level UNION missing CRITICAL finding: %+v", baseline.Findings) + } + for _, sql := range []string{ + "WITH c AS (" + unionSQL + ") SELECT * FROM c", + "SELECT * FROM (" + unionSQL + ") s", + } { + tree, err := parser.ParseWithDialect(sql, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + result := scanner.Scan(tree) + ast.ReleaseAST(tree) + if !reflect.DeepEqual(result.Findings, baseline.Findings) { + t.Errorf("nested UNION findings = %+v, want top-level findings %+v", result.Findings, baseline.Findings) + } + } + tree, err := parser.ParseWithDialect("SELECT * FROM t1 UNION ALL SELECT * FROM t2", keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + result := scanner.Scan(tree) + for _, pattern := range []PatternType{PatternTimeBased, PatternOutOfBand, PatternTautology} { + if hasPattern(result, pattern) { + t.Errorf("benign UNION has unexpected %s: %+v", pattern, result.Findings) + } + } + // The existing AST path produces no generic UNION finding. + if !result.IsClean() { + t.Errorf("benign UNION findings changed: %+v", result.Findings) + } +} + +func TestScan_ASTCoverageConcurrent(t *testing.T) { + scanner := NewScanner() + sqls := []string{ + "WITH c AS (SELECT pg_sleep(5)) SELECT * FROM c", + "SELECT CASE WHEN 1=1 THEN pg_sleep(5) END FROM t", + "SELECT * FROM (VALUES (pg_sleep(5))) v", + "SELECT id FROM t UNION SELECT table_name FROM information_schema.tables", + } + trees := make([]*ast.AST, 0, len(sqls)) + expected := make([]*ScanResult, 0, len(sqls)) + for _, sql := range sqls { + tree, err := parser.ParseWithDialect(sql, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + trees = append(trees, tree) + expected = append(expected, scanner.Scan(tree)) + } + var wg sync.WaitGroup + for range 8 { + wg.Add(1) + go func() { + defer wg.Done() + for range 10 { + for i, tree := range trees { + if result := scanner.Scan(tree); !reflect.DeepEqual(result, expected[i]) { + t.Errorf("concurrent Scan changed result for %q: %+v", sqls[i], result) + } + } + } + }() + } + wg.Wait() +} diff --git a/pkg/sql/security/scanner.go b/pkg/sql/security/scanner.go index f7e39094a..c008862e6 100644 --- a/pkg/sql/security/scanner.go +++ b/pkg/sql/security/scanner.go @@ -627,9 +627,88 @@ func (s *Scanner) Scan(tree *ast.AST) *ScanResult { return result } - for _, stmt := range tree.Statements { - s.scanStatement(stmt, result) - } + // Ancestry is local to the scan, so concurrent scans share no walk state. + // Preallocated so typical nesting depths need no regrowth. + ancestors := make([]ast.Node, 0, 16) + ast.Inspect(tree, func(node ast.Node) bool { + if node == nil { + ancestors = ancestors[:len(ancestors)-1] + return false + } + var parent ast.Node + if len(ancestors) > 0 { + parent = ancestors[len(ancestors)-1] + } + if join, ok := parent.(*ast.JoinClause); ok && node == &join.Left { + // Left repeats FROM or an earlier join's right reference, + // including its subquery and table function. + return false + } + switch n := node.(type) { + case *ast.FunctionCall: + s.scanFunctionCall(n, result) + s.scanCondition(n.Filter, result) + case *ast.TableReference: + s.scanTableReference(n, result) + case *ast.SetOperation: + // Direct operands are already checked by the outer operation. + // Operations inside CTEs and subqueries start a new check. + if _, nested := parent.(*ast.SetOperation); !nested { + s.checkUnionInjection(n, false, result) + } + case *ast.SelectStatement: + s.scanCondition(n.Where, result) + s.scanCondition(n.Having, result) + s.scanCondition(n.Qualify, result) + s.scanCondition(n.PrewhereClause, result) + s.scanCondition(n.StartWith, result) + case *ast.ConnectByClause: + s.scanCondition(n.Condition, result) + case *ast.JoinClause: + s.scanCondition(n.Condition, result) + s.scanCondition(n.MatchCondition, result) + case *ast.UpdateStatement: + s.scanCondition(n.Where, result) + case *ast.Update: + s.scanCondition(n.Where, result) + case *ast.DeleteStatement: + s.scanCondition(n.Where, result) + case *ast.Delete: + s.scanCondition(n.Where, result) + case *ast.MergeStatement: + s.scanCondition(n.OnCondition, result) + case *ast.MergeWhenClause: + s.scanCondition(n.Condition, result) + case *ast.MergeAction: + s.scanCondition(n.Where, result) + s.scanCondition(n.DeleteWhere, result) + case *ast.WhenClause: + s.scanCondition(n.Condition, result) + case *ast.OnConflict: + s.scanCondition(n.TargetWhere, result) + s.scanCondition(n.Action.Where, result) + case *ast.Values: + s.scanStatementText(n.SQL(), result) + return false + case *ast.UtilityStatement: + s.scanStatementText(n.Text, result) + return false + case *ast.SetStatement: + s.scanStatementText(n.Text, result) + return false + case *ast.TransactionStatement: + s.scanStatementText(n.Text, result) + return false + case *ast.UseStatement: + s.scanStatementText(n.Text, result) + return false + case *ast.UnsupportedStatement: + s.scanStatementText(n.RawSQL, result) + return false + } + ancestors = append(ancestors, node) + return true + }) // Update counts s.updateCounts(result) @@ -723,36 +802,6 @@ func (s *Scanner) scanSQL(sql string, prefilter bool) *ScanResult { return result } -// scanStatement analyzes a single statement for injection patterns. -func (s *Scanner) scanStatement(stmt ast.Statement, result *ScanResult) { - switch st := stmt.(type) { - case *ast.SelectStatement: - s.scanSelectStatement(st, result) - case *ast.InsertStatement: - s.scanInsertStatement(st, result) - case *ast.UpdateStatement: - s.scanUpdateStatement(st, result) - case *ast.DeleteStatement: - s.scanDeleteStatement(st, result) - case *ast.SetOperation: - s.scanSetOperation(st, result) - case *ast.Values: - // The text rules cover expressions under every VALUES wrapper (CAST, - // CASE, subqueries, ...), without duplicating AST function findings. - s.scanStatementText(st.SQL(), result) - case *ast.UtilityStatement: - s.scanStatementText(st.Text, result) - case *ast.SetStatement: - s.scanStatementText(st.Text, result) - case *ast.TransactionStatement: - s.scanStatementText(st.Text, result) - case *ast.UseStatement: - s.scanStatementText(st.Text, result) - case *ast.UnsupportedStatement: - s.scanStatementText(st.RawSQL, result) - } -} - // scanStatementText covers statements whose bodies are retained as text rather // than expression nodes, including PREPARE/EXECUTE, T-SQL blocks and CREATE // POLICY/TRIGGER. These nodes take only the text path, so AST and text checks @@ -762,38 +811,10 @@ func (s *Scanner) scanStatementText(text string, result *ScanResult) { result.Findings = append(result.Findings, findings.Findings...) } -// scanSelectStatement analyzes SELECT for injection patterns. -func (s *Scanner) scanSelectStatement(stmt *ast.SelectStatement, result *ScanResult) { - // Check WHERE clause for tautologies - if stmt.Where != nil { - s.scanExpression(stmt.Where, result, "WHERE clause") - } - - // Check HAVING clause - if stmt.Having != nil { - s.scanExpression(stmt.Having, result, "HAVING clause") - } - - // Check for suspicious function calls in columns - for _, col := range stmt.Columns { - s.scanExpressionForDangerousFunctions(col, result) - } - - // Check table-function references in FROM and JOIN. Joins[i].Left - // repeats a reference already in From or an earlier join, so only the - // right side is scanned. - for i := range stmt.From { - s.scanTableReference(&stmt.From[i], result) - } - for i := range stmt.Joins { - s.scanTableReference(&stmt.Joins[i].Right, result) - } -} - // scanTableReference checks a function-call table reference such as // ClickHouse FROM url(...) or PostgreSQL FROM generate_series(...): the -// function name against the out-of-band table functions, and the call and -// its arguments against the dangerous function lists. +// function name is checked against the out-of-band table functions. Inspect +// separately visits the FunctionCall and its arguments. func (s *Scanner) scanTableReference(ref *ast.TableReference, result *ScanResult) { if ref == nil || ref.TableFunc == nil { return @@ -810,98 +831,23 @@ func (s *Scanner) scanTableReference(ref *ast.TableReference, result *ScanResult result.Findings = append(result.Findings, finding) } } - s.scanFunctionCall(ref.TableFunc, result) -} - -// scanInsertStatement analyzes INSERT for injection patterns. -func (s *Scanner) scanInsertStatement(stmt *ast.InsertStatement, result *ScanResult) { - // Check values for suspicious patterns (multi-row support) - for _, row := range stmt.Values { - for _, val := range row { - s.scanExpressionForDangerousFunctions(val, result) - } - } -} - -// scanUpdateStatement analyzes UPDATE for injection patterns. -func (s *Scanner) scanUpdateStatement(stmt *ast.UpdateStatement, result *ScanResult) { - // Check WHERE clause - if stmt.Where != nil { - s.scanExpression(stmt.Where, result, "WHERE clause") - } - - // Check SET values - for _, assignment := range stmt.Assignments { - s.scanExpressionForDangerousFunctions(assignment.Value, result) - } } -// scanDeleteStatement analyzes DELETE for injection patterns. -func (s *Scanner) scanDeleteStatement(stmt *ast.DeleteStatement, result *ScanResult) { - // Check WHERE clause - if stmt.Where != nil { - s.scanExpression(stmt.Where, result, "WHERE clause") - } -} - -// scanSetOperation analyzes UNION/EXCEPT/INTERSECT for injection patterns. -// -// Parenthesized operands produce nested SetOperation trees on either side -// (e.g. "a UNION (b UNION c)"), so the whole tree is walked once for UNION -// operands and once for the per-statement checks of each leaf. -func (s *Scanner) scanSetOperation(stmt *ast.SetOperation, result *ScanResult) { - // UNION-based injection detection over the whole set-operation tree. - s.checkUnionInjection(stmt, false, result) - - // Scan every leaf statement of the tree. - s.scanSetOperationLeaves(stmt, result) -} - -// scanSetOperationLeaves runs the per-statement checks on each non-set-operation -// operand of stmt, descending into nested set operations. -func (s *Scanner) scanSetOperationLeaves(stmt *ast.SetOperation, result *ScanResult) { - for _, operand := range [2]ast.Statement{stmt.Left, stmt.Right} { - switch op := operand.(type) { - case nil: - case *ast.SetOperation: - s.scanSetOperationLeaves(op, result) - default: - s.scanStatement(op, result) - } - } -} - -// scanExpression analyzes an expression for injection patterns. -func (s *Scanner) scanExpression(expr ast.Expression, result *ScanResult, context string) { - if expr == nil { - return - } - +// scanCondition checks tautologies through boolean expression structure only. +// Inspect separately handles functions and nested statements. +func (s *Scanner) scanCondition(expr ast.Expression, result *ScanResult) { switch e := expr.(type) { case *ast.BinaryExpression: - s.scanBinaryExpression(e, result, context) - case *ast.FunctionCall: - s.scanFunctionCall(e, result) + s.scanBinaryExpression(e, result) case *ast.UnaryExpression: - if e.Expr != nil { - s.scanExpression(e.Expr, result, context) - } + s.scanCondition(e.Expr, result) case *ast.ParenthesizedExpression: - s.scanExpression(e.Expr, result, context) - case *ast.FieldAccessExpression: - s.scanExpression(e.Expr, result, context) - case *ast.ArraySubscriptExpression, *ast.ArraySliceExpression: - // A subscript or slice on a call result, such as pg_sleep(5)[1]. - s.scanExpressionForDangerousFunctions(e, result) - case *ast.MatchAgainstExpression, *ast.AssignmentExpression, *ast.IntervalExpression, - *ast.KeywordArgument, *ast.OuterJoinMarker, *ast.ConnectByRootExpression: - // Operands of MATCH ... AGAINST (SLEEP(5)), @v := SLEEP(5), ... - s.scanExpressionForDangerousFunctions(e, result) + s.scanCondition(e.Expr, result) } } // scanBinaryExpression checks for tautologies and suspicious patterns. -func (s *Scanner) scanBinaryExpression(expr *ast.BinaryExpression, result *ScanResult, context string) { +func (s *Scanner) scanBinaryExpression(expr *ast.BinaryExpression, result *ScanResult) { if expr == nil { return } @@ -926,8 +872,8 @@ func (s *Scanner) scanBinaryExpression(expr *ast.BinaryExpression, result *ScanR } // Recursively check sub-expressions - s.scanExpression(expr.Left, result, context) - s.scanExpression(expr.Right, result, context) + s.scanCondition(expr.Left, result) + s.scanCondition(expr.Right, result) } // isTautology checks if an expression is always true. @@ -960,7 +906,7 @@ func (s *Scanner) isTautology(expr *ast.BinaryExpression) bool { rightIdent, rightIsIdent := right.(*ast.Identifier) if leftIsIdent && rightIsIdent { - if leftIdent.Name == rightIdent.Name { + if leftIdent.Name == rightIdent.Name && leftIdent.Table == rightIdent.Table { return true } } @@ -1153,7 +1099,7 @@ var outOfBandTableFuncNames = map[string]string{ "EXECUTABLE": "Command execution", } -// scanFunctionCall checks for dangerous function usage. +// scanFunctionCall checks only this call name; Inspect visits its children. func (s *Scanner) scanFunctionCall(fn *ast.FunctionCall, result *ScanResult) { if fn == nil { return @@ -1204,56 +1150,6 @@ func (s *Scanner) scanFunctionCall(fn *ast.FunctionCall, result *ScanResult) { } } - // Recursively check function arguments - for _, arg := range fn.Arguments { - s.scanExpressionForDangerousFunctions(arg, result) - } -} - -// scanExpressionForDangerousFunctions recursively checks for dangerous functions. -func (s *Scanner) scanExpressionForDangerousFunctions(expr ast.Expression, result *ScanResult) { - if expr == nil { - return - } - - switch e := expr.(type) { - case *ast.FunctionCall: - s.scanFunctionCall(e, result) - case *ast.BinaryExpression: - s.scanExpressionForDangerousFunctions(e.Left, result) - s.scanExpressionForDangerousFunctions(e.Right, result) - case *ast.UnaryExpression: - s.scanExpressionForDangerousFunctions(e.Expr, result) - case *ast.ParenthesizedExpression: - s.scanExpressionForDangerousFunctions(e.Expr, result) - case *ast.FieldAccessExpression: - s.scanExpressionForDangerousFunctions(e.Expr, result) - case *ast.ArraySubscriptExpression: - s.scanExpressionForDangerousFunctions(e.Array, result) - for _, idx := range e.Indices { - s.scanExpressionForDangerousFunctions(idx, result) - } - case *ast.ArraySliceExpression: - s.scanExpressionForDangerousFunctions(e.Array, result) - s.scanExpressionForDangerousFunctions(e.Start, result) - s.scanExpressionForDangerousFunctions(e.End, result) - case *ast.KeywordArgument: - // SUBSTRING(s FROM SLEEP(5)), TRIM(BOTH SLEEP(5) FROM s) - s.scanExpressionForDangerousFunctions(e.Value, result) - case *ast.MatchAgainstExpression: - for _, c := range e.Columns { - s.scanExpressionForDangerousFunctions(c, result) - } - s.scanExpressionForDangerousFunctions(e.Query, result) - case *ast.AssignmentExpression: - s.scanExpressionForDangerousFunctions(e.Value, result) - case *ast.IntervalExpression: - s.scanExpressionForDangerousFunctions(e.ValueExpr, result) - case *ast.OuterJoinMarker: - s.scanExpressionForDangerousFunctions(e.Expr, result) - case *ast.ConnectByRootExpression: - s.scanExpressionForDangerousFunctions(e.Expr, result) - } } // detectCommentPatterns checks raw SQL for comment-based injection. folded is the From 21c71378a04a22b79359434577280017581e41ff Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Wed, 28 Oct 2026 11:10:08 +0530 Subject: [PATCH 03/10] fix(parser): reserve TOP in SQL Server names; parse full MariaDB DELETE HISTORY SQL Server: a delimited [top] / "top" is now an ordinary name (the select list no longer reads it as the TOP clause) and keeps its delimiters in SQL() and the formatter. Bare TOP is rejected where a table name, DML/MERGE target, table alias or select-item alias is expected, matching T-SQL, instead of being accepted in FROM/INSERT/DELETE but rejected in UPDATE. MariaDB: DELETE HISTORY FROM t [PARTITION (p, ...)] [BEFORE SYSTEM_TIME [TIMESTAMP | TRANSACTION] expr] parses any point expression and round-trips the partition list and unit keyword. The new fields live in DMLTargetFields.History (DeleteHistoryOptions) so DeleteStatement keeps its size class. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 10 ++ pkg/formatter/mariadb_delete_history_test.go | 42 ++++++ pkg/formatter/render.go | 20 +++ pkg/sql/ast/ast_dml.go | 17 ++- pkg/sql/ast/ast_statements.go | 7 + pkg/sql/ast/pool_dml_release.go | 5 + pkg/sql/ast/sql.go | 18 +++ pkg/sql/parser/ddl.go | 2 +- pkg/sql/parser/dml_common.go | 5 +- pkg/sql/parser/dml_delete.go | 35 ++++- pkg/sql/parser/dml_merge.go | 4 +- pkg/sql/parser/mariadb_delete_history_test.go | 101 +++++++++++++ pkg/sql/parser/name_quote.go | 18 ++- pkg/sql/parser/parser.go | 3 + pkg/sql/parser/pivot.go | 4 +- pkg/sql/parser/select.go | 8 +- pkg/sql/parser/select_subquery.go | 8 +- pkg/sql/parser/sqlserver_top_reserved_test.go | 133 ++++++++++++++++++ pkg/sql/parser/statement_boundary.go | 2 +- pkg/sql/parser/utility_stmt.go | 3 + 20 files changed, 425 insertions(+), 20 deletions(-) create mode 100644 pkg/formatter/mariadb_delete_history_test.go create mode 100644 pkg/sql/parser/mariadb_delete_history_test.go create mode 100644 pkg/sql/parser/sqlserver_top_reserved_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2935c0517..d9451f759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 in dialects without them. ### Fixed +- SQL Server: a delimited `[top]` / `"top"` is a plain name (also in the select + list, where it was read as the TOP clause) and keeps its delimiters on output; + bare `TOP` is rejected as a table, DML target or alias name, as T-SQL reserves it. +- MariaDB `DELETE HISTORY FROM t [PARTITION (p, ...)] [BEFORE SYSTEM_TIME + [TIMESTAMP | TRANSACTION] expr]` parses and round-trips the partition list, the + unit keyword and any point expression (e.g. `NOW() - INTERVAL 1 DAY`), held in + the new `ast.DMLTargetFields.History` (`ast.DeleteHistoryOptions`). - Watch now respects `--dialect`, re-validates changes and formats files in place with the CLI safety checks; MCP parsing accepts a dialect and documents comment preservation. - Correct transpile dialect help, avoid duplicate lint syntax errors, and rewrite @@ -202,6 +209,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 longer read as MariaDB's `ANALYZE `. ### Behavior changes +- SQL Server: unquoted `top` (any case) is now a syntax error where a table + name, INSERT/UPDATE/DELETE/MERGE target, table alias or select-item alias is + expected (`SELECT * FROM top`, `INSERT INTO top ...`); quote it as `[top]`. - Numeric words without whitespace no longer silently become a number plus a column alias. Numeric spelling is retained. Differences from the previous lexer: diff --git a/pkg/formatter/mariadb_delete_history_test.go b/pkg/formatter/mariadb_delete_history_test.go new file mode 100644 index 000000000..257cf5b68 --- /dev/null +++ b/pkg/formatter/mariadb_delete_history_test.go @@ -0,0 +1,42 @@ +package formatter_test + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestMariaDBDeleteHistoryFormatterRoundTrip(t *testing.T) { + for _, sql := range []string{ + `DELETE HISTORY FROM t PARTITION (p0, p1) BEFORE SYSTEM_TIME TRANSACTION 42`, + `DELETE HISTORY FROM t PARTITION (p0) BEFORE SYSTEM_TIME TIMESTAMP NOW() - INTERVAL 1 DAY`, + "DELETE HISTORY FROM t PARTITION (`select`) BEFORE SYSTEM_TIME '2020-01-01' + INTERVAL 1 DAY", + } { + t.Run(sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + lower := ast.CompactStyle() + lower.KeywordCase = ast.KeywordLower + formatted, err := gosqlx.FormatWithDialect(sql, gosqlx.DefaultFormatOptions(), keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + for _, output := range []string{formatted, formatter.FormatAST(tree, ast.CompactStyle()), formatter.FormatAST(tree, ast.ReadableStyle()), formatter.FormatAST(tree, lower)} { + again, err := gosqlx.ParseWithDialect(output, keywords.DialectMariaDB) + if err != nil { + t.Fatalf("reparse %q: %v", output, err) + } + defer ast.ReleaseAST(again) + if got := again.SQL(); got != sql { + t.Fatalf("formatted %q reparsed to %q, want %q", output, got, sql) + } + } + }) + } +} diff --git a/pkg/formatter/render.go b/pkg/formatter/render.go index e9631b36e..ce641957b 100644 --- a/pkg/formatter/render.go +++ b/pkg/formatter/render.go @@ -596,10 +596,30 @@ func renderDelete(d *ast.DeleteStatement, opts ast.FormatOptions) string { sb.WriteString(" ") } writeDMLTargets(sb, f, x, d.TableName, d.TableNameQuote, d.Alias, d.AliasQuote) + var history ast.DeleteHistoryOptions + if x.History != nil { + history = *x.History + } + if len(history.Partitions) > 0 { + sb.WriteString(" ") + sb.WriteString(f.kw("PARTITION")) + sb.WriteString(" (") + for i, partition := range history.Partitions { + if i > 0 { + sb.WriteString(", ") + } + sb.WriteString(f.exprSQL(partition)) + } + sb.WriteByte(')') + } if d.BeforeSystemTime != nil { sb.WriteString(" ") sb.WriteString(f.kw("BEFORE SYSTEM_TIME")) sb.WriteString(" ") + if history.BeforeSystemTimeUnit != "" { + sb.WriteString(f.kw(history.BeforeSystemTimeUnit)) + sb.WriteString(" ") + } sb.WriteString(f.exprSQL(d.BeforeSystemTime)) } writeDMLOutput(sb, f, x.Output, x.OutputInto) diff --git a/pkg/sql/ast/ast_dml.go b/pkg/sql/ast/ast_dml.go index 9ea07b439..e05d2dcce 100644 --- a/pkg/sql/ast/ast_dml.go +++ b/pkg/sql/ast/ast_dml.go @@ -50,6 +50,21 @@ type DMLTargetFields struct { Limit Expression // ReturningInto holds the Oracle RETURNING ... INTO :bind targets. ReturningInto []Expression + // History holds the MariaDB DELETE HISTORY partition list and BEFORE + // SYSTEM_TIME unit; nil for every other statement. + History *DeleteHistoryOptions +} + +// DeleteHistoryOptions holds the optional parts of MariaDB's +// DELETE HISTORY FROM t [PARTITION (p, ...)] [BEFORE SYSTEM_TIME [unit] point] +// other than the point itself (DeleteStatement.BeforeSystemTime). It lives in +// DMLTargetFields so the rarely used fields do not grow DeleteStatement. +type DeleteHistoryOptions struct { + // Partitions are the PARTITION (...) names, in source order. + Partitions []*Identifier + // BeforeSystemTimeUnit is "TIMESTAMP" or "TRANSACTION" when written + // before the point, and "" otherwise. + BeforeSystemTimeUnit string } // dmlExtras returns a copy of x, or a zero value when it is nil, so renderers @@ -91,7 +106,7 @@ func (d *DeleteStatement) EnsureExtras() *DMLTargetFields { func (d *DMLTargetFields) IsZero() bool { return len(d.Modifiers) == 0 && d.Top == nil && !d.Only && len(d.Tables) == 0 && len(d.Joins) == 0 && len(d.Output) == 0 && d.OutputInto == nil && - len(d.OrderBy) == 0 && d.Limit == nil && len(d.ReturningInto) == 0 + len(d.OrderBy) == 0 && d.Limit == nil && len(d.ReturningInto) == 0 && d.History == nil } // dmlTargetChildren appends the nodes of d to children. diff --git a/pkg/sql/ast/ast_statements.go b/pkg/sql/ast/ast_statements.go index d81ea794e..66492955c 100644 --- a/pkg/sql/ast/ast_statements.go +++ b/pkg/sql/ast/ast_statements.go @@ -524,6 +524,8 @@ type DeleteStatement struct { // BeforeSystemTime is the point of MariaDB's DELETE HISTORY FROM t // BEFORE SYSTEM_TIME point; nil when the statement has none. DELETE // HISTORY itself is the HISTORY modifier. + // The PARTITION list and the TIMESTAMP / TRANSACTION unit of the point + // are in Extras.History. BeforeSystemTime Expression // TableNameQuote and AliasQuote record the delimiters of TableName and // Alias when they must be kept (see NameQuote). @@ -549,6 +551,11 @@ func (d DeleteStatement) Children() []Node { for i := range d.From { children = append(children, &d.From[i]) } + if d.Extras != nil && d.Extras.History != nil { + for _, partition := range d.Extras.History.Partitions { + children = append(children, partition) + } + } if d.BeforeSystemTime != nil { children = append(children, d.BeforeSystemTime) } diff --git a/pkg/sql/ast/pool_dml_release.go b/pkg/sql/ast/pool_dml_release.go index e0e4f0e49..af3e25c79 100644 --- a/pkg/sql/ast/pool_dml_release.go +++ b/pkg/sql/ast/pool_dml_release.go @@ -47,6 +47,11 @@ func releaseDMLTargetFields(d *DMLTargetFields, mode releaseMode) { for _, e := range d.ReturningInto { releaseExpression(e, mode) } + if d.History != nil { + for _, partition := range d.History.Partitions { + releaseExpression(partition, mode) + } + } *d = DMLTargetFields{} } diff --git a/pkg/sql/ast/sql.go b/pkg/sql/ast/sql.go index 9975dc255..cd4242ef4 100644 --- a/pkg/sql/ast/sql.go +++ b/pkg/sql/ast/sql.go @@ -1578,8 +1578,26 @@ func (d *DeleteStatement) writeSQL(buf *bytes.Buffer) { buf.WriteString("FROM ") } x.writeDMLTargets(buf, d.TableName, d.TableNameQuote, d.Alias, d.AliasQuote) + var history DeleteHistoryOptions + if x.History != nil { + history = *x.History + } + if len(history.Partitions) > 0 { + buf.WriteString(" PARTITION (") + for i, partition := range history.Partitions { + if i > 0 { + buf.WriteString(", ") + } + partition.writeSQL(buf) + } + buf.WriteByte(')') + } if d.BeforeSystemTime != nil { buf.WriteString(" BEFORE SYSTEM_TIME ") + if history.BeforeSystemTimeUnit != "" { + buf.WriteString(history.BeforeSystemTimeUnit) + buf.WriteByte(' ') + } writeExpr(buf, d.BeforeSystemTime) } writeDMLOutput(buf, x.Output, x.OutputInto) diff --git a/pkg/sql/parser/ddl.go b/pkg/sql/parser/ddl.go index c64b09c5a..c01ee8469 100644 --- a/pkg/sql/parser/ddl.go +++ b/pkg/sql/parser/ddl.go @@ -202,7 +202,7 @@ func (p *Parser) parseCreateTable(temporary bool) (*ast.CreateTableStatement, er } // Parse table name (supports schema.table qualification and double-quoted identifiers) - createTableName, createTableQuote, err := p.parseQualifiedNameQuoted() + createTableName, createTableQuote, err := p.parseQualifiedNameWithTableRules(true) if err != nil { return nil, p.qualifiedNameError(err, "table name") } diff --git a/pkg/sql/parser/dml_common.go b/pkg/sql/parser/dml_common.go index d8180ac8a..effe83726 100644 --- a/pkg/sql/parser/dml_common.go +++ b/pkg/sql/parser/dml_common.go @@ -155,6 +155,9 @@ func (p *Parser) isDMLOutputKeywordAt() bool { // isDMLTargetAlias reports whether the current token is a bare alias after // an UPDATE/DELETE target name rather than the clause that follows it. func (p *Parser) isDMLTargetAlias() bool { + if p.isReservedTopName() { + return false + } t := p.currentToken.Token.Type return (t == models.TokenTypeIdentifier || t == models.TokenTypeDoubleQuotedString) && p.isDMLTargetAliasAt() } @@ -190,7 +193,7 @@ func (p *Parser) parseDMLTargetName(name *string, nameQuote *ast.NameQuote, alia *name, *nameQuote = n, q if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() { + if !p.isIdentifier() || p.isReservedTopName() { return p.expectedError("alias after AS") } *alias, *aliasQuote = p.currentToken.Token.Value, p.currentNameQuote() diff --git a/pkg/sql/parser/dml_delete.go b/pkg/sql/parser/dml_delete.go index c3c7bfc58..a700d558e 100644 --- a/pkg/sql/parser/dml_delete.go +++ b/pkg/sql/parser/dml_delete.go @@ -179,7 +179,8 @@ func (p *Parser) parseDeleteSources(dst *[]ast.TableReference, joinsDst *[]ast.J } // parseDeleteHistory parses the rest of MariaDB's DELETE HISTORY FROM table -// [BEFORE SYSTEM_TIME point]; the current token is HISTORY. HISTORY is kept as +// [PARTITION (partition_list)] [BEFORE SYSTEM_TIME [TIMESTAMP | TRANSACTION] expr]; +// the current token is HISTORY. HISTORY is kept as // a statement modifier. func (p *Parser) parseDeleteHistory(stmt *ast.DeleteStatement, x *ast.DMLTargetFields) (ast.Statement, error) { x.Modifiers = append(x.Modifiers, "HISTORY") @@ -190,13 +191,43 @@ func (p *Parser) parseDeleteHistory(stmt *ast.DeleteStatement, x *ast.DMLTargetF return nil, err } stmt.TableName, stmt.TableNameQuote = name, quote + var history ast.DeleteHistoryOptions + if p.isType(models.TokenTypePartition) { + p.advance() // Consume PARTITION + if !p.matchType(models.TokenTypeLParen) { + return nil, p.expectedError("(") + } + for { + if !p.isIdentifier() && !p.isNonReservedKeyword() { + return nil, p.expectedError("partition name") + } + partition := p.parseColumnName() + if partition == nil { + return nil, p.expectedError("partition name") + } + history.Partitions = append(history.Partitions, partition) + if !p.matchType(models.TokenTypeComma) { + break + } + } + if !p.matchType(models.TokenTypeRParen) { + return nil, p.expectedError(")") + } + } if p.isUnquotedWord("BEFORE") && p.peekValueIs(1, "SYSTEM_TIME") { p.advance() // Consume BEFORE p.advance() // Consume SYSTEM_TIME - if stmt.BeforeSystemTime, err = p.parseTemporalPointExpression(); err != nil { + if p.isUnquotedWord("TIMESTAMP") || p.isUnquotedWord("TRANSACTION") { + history.BeforeSystemTimeUnit = strings.ToUpper(p.currentToken.Token.Value) + p.advance() + } + if stmt.BeforeSystemTime, err = p.parseExpression(); err != nil { return nil, err } } + if len(history.Partitions) > 0 || history.BeforeSystemTimeUnit != "" { + x.History = &history + } stmt.Extras = finishDMLExtras(x) return stmt, nil } diff --git a/pkg/sql/parser/dml_merge.go b/pkg/sql/parser/dml_merge.go index 91afa6e86..2327b9887 100644 --- a/pkg/sql/parser/dml_merge.go +++ b/pkg/sql/parser/dml_merge.go @@ -50,7 +50,7 @@ func (p *Parser) parseMergeStatement() (ast.Statement, error) { // Parse optional target alias (AS alias or just alias) if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() && !p.isNonReservedKeyword() { + if p.isReservedTopName() || (!p.isIdentifier() && !p.isNonReservedKeyword()) { return nil, p.expectedError("target alias after AS") } stmt.TargetAlias = p.currentToken.Token.Value @@ -89,7 +89,7 @@ func (p *Parser) parseMergeStatement() (ast.Statement, error) { // Parse optional source alias if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() && !p.isNonReservedKeyword() { + if p.isReservedTopName() || (!p.isIdentifier() && !p.isNonReservedKeyword()) { return nil, p.expectedError("source alias after AS") } stmt.SourceAlias = p.currentToken.Token.Value diff --git a/pkg/sql/parser/mariadb_delete_history_test.go b/pkg/sql/parser/mariadb_delete_history_test.go new file mode 100644 index 000000000..2a02fd207 --- /dev/null +++ b/pkg/sql/parser/mariadb_delete_history_test.go @@ -0,0 +1,101 @@ +package parser_test + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestMariaDBDeleteHistoryRoundTrip(t *testing.T) { + cases := []string{ + `DELETE HISTORY FROM t`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01 00:00:00'`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TIMESTAMP '2020-01-01 00:00:00'`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME NOW() - INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME CURRENT_TIMESTAMP - INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01' + INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TIMESTAMP NOW() - INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TIMESTAMP '2020-01-01' + INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TRANSACTION 42`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME @ts`, + `DELETE HISTORY FROM db.t BEFORE SYSTEM_TIME '2020-01-01'`, + `DELETE HISTORY FROM t PARTITION (p0, p1)`, + `DELETE HISTORY FROM t PARTITION (p0) BEFORE SYSTEM_TIME '2020-01-01'`, + "DELETE HISTORY FROM t PARTITION (`select`)", + `delete history from t before system_time transaction 7`, + `DELETE FROM history WHERE a = 1`, + `SELECT * FROM t FOR SYSTEM_TIME AS OF TIMESTAMP '2020-01-01'`, + } + for _, sql := range cases { + t.Run(sql, func(t *testing.T) { + want := sql + if strings.HasPrefix(sql, "delete ") { + want = `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TRANSACTION 7` + } + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + if got := tree.SQL(); got != want { + t.Fatalf("SQL() = %q, want %q", got, want) + } + formatted, err := gosqlx.FormatWithDialect(sql, gosqlx.DefaultFormatOptions(), keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + for _, output := range []string{tree.SQL(), formatted} { + again, err := gosqlx.ParseWithDialect(output, keywords.DialectMariaDB) + if err != nil { + t.Fatalf("reparse %q: %v", output, err) + } + defer ast.ReleaseAST(again) + if got := again.SQL(); got != want { + t.Fatalf("round trip = %q, want %q", got, want) + } + } + }) + } +} + +func TestMariaDBDeleteHistoryRejectsInvalid(t *testing.T) { + for _, sql := range []string{ + `DELETE HISTORY FROM t WHERE a = 1`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01' WHERE a = 1`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01' LIMIT 5`, + `DELETE HISTORY FROM t LIMIT 5`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME`, + `DELETE HISTORY FROM t BEFORE '2020-01-01'`, + `DELETE HISTORY FROM t PARTITION ()`, + `DELETE HISTORY FROM t PARTITION (p0,)`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TRANSACTION`, + } { + t.Run(sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectMariaDB) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err == nil { + t.Fatal("expected syntax error") + } + }) + } +} + +func TestMySQLDeleteHistoryRemainsTarget(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(`DELETE HISTORY FROM t`, keywords.DialectMySQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + stmt, ok := tree.Statements[0].(*ast.DeleteStatement) + if !ok { + t.Fatalf("got %T", tree.Statements[0]) + } + if !strings.EqualFold(stmt.TableName, "history") || len(stmt.From) != 1 { + t.Fatalf("expected multi-table DELETE: %#v", stmt) + } +} diff --git a/pkg/sql/parser/name_quote.go b/pkg/sql/parser/name_quote.go index 20738edb9..33432b8fe 100644 --- a/pkg/sql/parser/name_quote.go +++ b/pkg/sql/parser/name_quote.go @@ -52,7 +52,9 @@ func (p *Parser) delimitedNameMustStayQuoted(name string, quote rune) bool { if quote == 0 { return false } - return ast.DelimitedNameMustStayQuoted(name, p.identifierCaseFolding()) || tokenizer.IsKeyword(name) || + // TOP is a dialect keyword, absent from the tokenizer's global keyword map. + return (p.IsSQLServer() && equalsUpper(name, "TOP")) || + ast.DelimitedNameMustStayQuoted(name, p.identifierCaseFolding()) || tokenizer.IsKeyword(name) || isContextualKeywordName(name) } @@ -142,6 +144,18 @@ func (p *Parser) appendNamePartQuoted(q ast.NameQuote, quote rune) ast.NameQuote // parseQualifiedNameQuoted parses a possibly qualified object name like // parseQualifiedName and also returns how its parts were delimited. func (p *Parser) parseQualifiedNameQuoted() (string, ast.NameQuote, error) { + return p.parseQualifiedNameWithTableRules(false) +} + +// isReservedTopName checks the dialect before inspecting the token text. +func (p *Parser) isReservedTopName() bool { + return p.IsSQLServer() && p.isUnquotedWord("TOP") +} + +func (p *Parser) parseQualifiedNameWithTableRules(table bool) (string, ast.NameQuote, error) { + if table && p.isReservedTopName() { + return "", 0, p.expectedError("table name (TOP is reserved in SQL Server; quote it as [top])") + } if !p.isIdentifier() && !p.isNonReservedKeyword() { return "", 0, p.expectedError("identifier") } @@ -176,7 +190,7 @@ func (p *Parser) parseQualifiedNameQuoted() (string, ast.NameQuote, error) { "", ).WithCause(errQualifiedNameTooLong) } - if !p.isIdentifier() && !p.isNonReservedKeyword() { + if (table && p.isReservedTopName()) || (!p.isIdentifier() && !p.isNonReservedKeyword()) { return "", 0, p.expectedError("identifier after .") } parts[n] = p.currentToken.Token.Value diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go index 6f10dbbb6..94fa4e2c9 100644 --- a/pkg/sql/parser/parser.go +++ b/pkg/sql/parser/parser.go @@ -1400,6 +1400,9 @@ func (p *Parser) nextStartsTableClause() bool { // but NOT contextual clause keywords that would be consumed as aliases by mistake // (e.g. MINUS in Snowflake/Oracle, QUALIFY in Snowflake/BigQuery, WINDOW). func (p *Parser) canBeAlias() bool { + if p.isReservedTopName() { + return false + } // Neither isIdentifier nor isNonReservedKeyword holds at an operand end, // such as the "," or FROM after a select item. if p.atOperandEnd() { diff --git a/pkg/sql/parser/pivot.go b/pkg/sql/parser/pivot.go index abedcdf72..f50642d48 100644 --- a/pkg/sql/parser/pivot.go +++ b/pkg/sql/parser/pivot.go @@ -82,14 +82,14 @@ func (p *Parser) renderPivotValue() string { func (p *Parser) parsePivotAlias(ref *ast.TableReference) { if p.isType(models.TokenTypeAs) { p.advance() // consume AS - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { ref.Alias = p.currentToken.Token.Value ref.AliasQuote = p.currentNameQuote() p.advance() } return } - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { ref.Alias = p.currentToken.Token.Value ref.AliasQuote = p.currentNameQuote() p.advance() diff --git a/pkg/sql/parser/select.go b/pkg/sql/parser/select.go index 4529fc684..e8bb09100 100644 --- a/pkg/sql/parser/select.go +++ b/pkg/sql/parser/select.go @@ -57,7 +57,7 @@ func (p *Parser) parseSelectStatement() (ast.Statement, error) { // Reject TOP in dialects that use LIMIT/OFFSET or ROWNUM/FETCH FIRST instead. // String comparison is used here because the lexer has no TokenTypeTOP constant; // TOP is emitted as an identifier literal (see parseTopClause comment). - if p.isNonTopDialect() && equalsUpper(p.currentToken.Token.Value, "TOP") { + if p.isNonTopDialect() && p.isUnquotedWord("TOP") { if p.dialect == string(keywords.DialectOracle) { return nil, goerrors.UnsupportedFeatureError( "TOP clause is not supported in Oracle; use ROWNUM or FETCH FIRST … ROWS ONLY instead", @@ -441,16 +441,16 @@ func (p *Parser) isNonTopDialect() bool { // Note: "TOP" is detected via a string comparison rather than a dedicated token-type // constant because the lexer does not define a TokenTypeTOP - it tokenises TOP as a // plain identifier/keyword literal. A future lexer enhancement could introduce -// models.TokenTypeTop and replace the equalsUpper check below. +// models.TokenTypeTop and replace the word check below. func (p *Parser) parseTopClause() (*ast.TopClause, error) { switch p.dialect { case string(keywords.DialectSQLServer): - if !equalsUpper(p.currentToken.Token.Value, "TOP") { + if !p.isUnquotedWord("TOP") { return nil, nil } case string(keywords.DialectSnowflake): // Snowflake TOP ; a column named top is left alone. - if !equalsUpper(p.currentToken.Token.Value, "TOP") || p.peekToken().Token.Type != models.TokenTypeNumber { + if !p.isUnquotedWord("TOP") || p.peekToken().Token.Type != models.TokenTypeNumber { return nil, nil } default: diff --git a/pkg/sql/parser/select_subquery.go b/pkg/sql/parser/select_subquery.go index 2d23b3a15..05ec905e0 100644 --- a/pkg/sql/parser/select_subquery.go +++ b/pkg/sql/parser/select_subquery.go @@ -169,11 +169,11 @@ func (p *Parser) parseFromTableReference() (ast.TableReference, error) { if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() && !p.isDMLReturningKeyword() && !p.isSelectExtClauseStart() && !p.isTableModifierAfterAliasStart() && !p.isTSQLStatementStart() { if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() { + if !p.isIdentifier() || p.isReservedTopName() { return tableRef, p.expectedError("alias after AS") } } - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { tableRef.Alias = p.currentToken.Token.Value tableRef.AliasQuote = p.currentNameQuote() p.advance() @@ -362,11 +362,11 @@ func (p *Parser) parseJoinedTableRef(joinType string) (ast.TableReference, error if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() && !p.isDMLReturningKeyword() && !p.isSelectExtClauseStart() && !p.isTableModifierAfterAliasStart() && !p.isTSQLStatementStart() { if p.isType(models.TokenTypeAs) { p.advance() - if !p.isIdentifier() { + if !p.isIdentifier() || p.isReservedTopName() { return ref, p.expectedError("alias after AS") } } - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { ref.Alias = p.currentToken.Token.Value ref.AliasQuote = p.currentNameQuote() p.advance() diff --git a/pkg/sql/parser/sqlserver_top_reserved_test.go b/pkg/sql/parser/sqlserver_top_reserved_test.go new file mode 100644 index 000000000..8208c9391 --- /dev/null +++ b/pkg/sql/parser/sqlserver_top_reserved_test.go @@ -0,0 +1,133 @@ +package parser_test + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestSQLServerTopReserved(t *testing.T) { + for _, sql := range []string{ + "INSERT INTO top (a) VALUES (1)", "INSERT INTO TOP (a) VALUES (1)", "INSERT INTO Top (a) VALUES (1)", + "SELECT * FROM top", "SELECT * FROM TOP", "DELETE FROM top", "UPDATE top SET a = 1", + "SELECT * FROM t AS top", "SELECT * FROM t top", "SELECT a AS top FROM t", "SELECT 1 top FROM t", "SELECT a top FROM t", + "CREATE TABLE top (a INT)", "SELECT * FROM dbo.top", "MERGE INTO top USING s ON s.a = top.a WHEN MATCHED THEN DELETE", + "SELECT * FROM t PIVOT (SUM(a) FOR q IN ([Q1])) AS top", + "SELECT * FROM t PIVOT (SUM(a) FOR q IN ([Q1])) top", + "SELECT * FROM t JOIN s AS top ON t.a = s.a", "SELECT * FROM t JOIN s top ON t.a = s.a", + "UPDATE t AS top SET a = 1", "DELETE FROM t AS top", + } { + t.Run(sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectSQLServer) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err == nil { + t.Fatal("expected syntax error") + } + }) + } +} + +func TestSQLServerTopQuotedRoundTrip(t *testing.T) { + for _, tc := range []struct{ sql, want string }{ + {"SELECT * FROM [top]", "SELECT * FROM [top]"}, {`SELECT * FROM "top"`, `SELECT * FROM "top"`}, + {"SELECT [top] FROM t", "SELECT [top] FROM t"}, {`SELECT "top" FROM t`, `SELECT "top" FROM t`}, + {"SELECT a AS [top] FROM t", "SELECT a AS [top] FROM t"}, {"SELECT * FROM t AS [top]", "SELECT * FROM t [top]"}, + {"INSERT INTO [top] (a) VALUES (1)", "INSERT INTO [top] (a) VALUES (1)"}, {"UPDATE [top] SET a = 1", "UPDATE [top] SET a = 1"}, + {"SELECT top 5 * FROM t", "SELECT TOP 5 * FROM t"}, {"INSERT top (5) INTO t SELECT * FROM s", "INSERT TOP (5) INTO t SELECT * FROM s"}, + {"INSERT Top (5) t (a) VALUES (1)", "INSERT TOP (5) INTO t (a) VALUES (1)"}, {"UPDATE top (5) t SET a = 1", "UPDATE TOP (5) t SET a = 1"}, + {"DELETE top (5) FROM t", "DELETE TOP (5) FROM t"}, {"SELECT t.top FROM t", "SELECT t.top FROM t"}, {"SELECT * FROM [select]", "SELECT * FROM [select]"}, + {"CREATE TABLE t (top INT)", "CREATE TABLE t (top INT)"}, + } { + t.Run(tc.sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(tc.sql, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + got := tree.SQL() + if got != tc.want { + t.Fatalf("got %q want %q", got, tc.want) + } + again, err := gosqlx.ParseWithDialect(got, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(again) + if again.SQL() != got { + t.Fatal("unstable SQL") + } + formatted, err := gosqlx.FormatWithDialect(tc.sql, gosqlx.DefaultFormatOptions(), keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + if strings.Contains(tc.sql, "[top]") && !strings.Contains(formatted, "[top]") { + t.Fatalf("lost brackets: %s", formatted) + } + ft, err := gosqlx.ParseWithDialect(formatted, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(ft) + if ft.SQL() != got { + t.Fatalf("formatter changed SQL: %s", ft.SQL()) + } + }) + } +} + +func TestTopNamesOtherDialects(t *testing.T) { + for _, dialect := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectMariaDB, keywords.DialectSQLite, keywords.DialectOracle, keywords.DialectClickHouse, keywords.DialectGeneric, keywords.DialectSnowflake} { + for _, sql := range []string{"INSERT INTO top (a) VALUES (1)", "SELECT * FROM top"} { + t.Run(string(dialect)+sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + if tree.SQL() != sql { + t.Fatalf("SQL() = %q, want %q", tree.SQL(), sql) + } + again, err := gosqlx.ParseWithDialect(tree.SQL(), dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(again) + if tree.SQL() != again.SQL() { + t.Fatal("unstable SQL") + } + formatted, err := gosqlx.FormatWithDialect(sql, gosqlx.DefaultFormatOptions(), dialect) + if err != nil { + t.Fatal(err) + } + ft, err := gosqlx.ParseWithDialect(formatted, dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(ft) + if ft.SQL() != sql { + t.Fatalf("formatter changed SQL: %s", ft.SQL()) + } + }) + } + } +} + +// Quoted TOP must be an expression even in dialects that reject the TOP clause. +func TestDelimitedTopIsNotClause(t *testing.T) { + for _, dialect := range []keywords.SQLDialect{keywords.DialectGeneric, keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectMariaDB, keywords.DialectSQLite, keywords.DialectOracle, keywords.DialectClickHouse, keywords.DialectSnowflake} { + for _, sql := range []string{`SELECT "top" FROM t`} { + t.Run(string(dialect)+sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + }) + } + } +} diff --git a/pkg/sql/parser/statement_boundary.go b/pkg/sql/parser/statement_boundary.go index 5a9775777..7f692b4cb 100644 --- a/pkg/sql/parser/statement_boundary.go +++ b/pkg/sql/parser/statement_boundary.go @@ -222,5 +222,5 @@ func (p *Parser) parseTableNameQuoted() (string, ast.NameQuote, error) { p.advance() return name, 0, nil } - return p.parseQualifiedNameQuoted() + return p.parseQualifiedNameWithTableRules(true) } diff --git a/pkg/sql/parser/utility_stmt.go b/pkg/sql/parser/utility_stmt.go index 3cbda0ddc..50af4bfda 100644 --- a/pkg/sql/parser/utility_stmt.go +++ b/pkg/sql/parser/utility_stmt.go @@ -985,6 +985,9 @@ func (p *Parser) currentIdentQuote() rune { // follow. Words that start a clause or a set operation are still rejected, // so SELECT a AS FROM t keeps its error. func (p *Parser) isAliasAfterAS() bool { + if p.isReservedTopName() { + return false + } if p.isIdentifier() || p.isNonReservedKeyword() { return true } From 34792872c4ec7fc170c6670054dd618ca9554b29 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Wed, 28 Oct 2026 17:45:41 +0530 Subject: [PATCH 04/10] fix(parser,tokenizer): stop GRANT cursor panic; MySQL/MariaDB # line comments - utility grammar cursor no longer stores the -1 groupEnd/bracketEnd sentinel before failing (GRANT ... TO [ / [AppUser / CURRENT_USER(, MariaDB IDENTIFIED VIA ... USING PASSWORD(, COPY (); Parser.seek clamps negative indexes to EOF. These inputs now return E2002 instead of panicking. - With the MySQL or MariaDB dialect, '#' starts a line comment in the tokenizer (recorded in Comments, preserved by the formatter), the statement splitter and its procedural lookahead, highlight error recovery and the dropped-word checker. Other dialects keep their '#' meaning. - Regression tests over all 8 dialects, fuzz seeds, realworld corpus cases; literal golden regenerated (only mysql/mariadb digests change for inputs containing '#'). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2 --- CHANGELOG.md | 11 + internal/dropcheck/dropcheck.go | 13 +- internal/dropcheck/dropcheck_test.go | 28 +++ pkg/gosqlx/hash_comments_test.go | 110 +++++++++ pkg/gosqlx/splitter.go | 7 +- pkg/gosqlx/splitter_proc.go | 22 +- pkg/highlight/highlight.go | 3 +- pkg/highlight/highlight_test.go | 28 +++ pkg/sql/parser/fuzz_test.go | 7 + pkg/sql/parser/utility_cursor_test.go | 121 ++++++++++ pkg/sql/parser/utility_grammar.go | 20 +- pkg/sql/parser/utility_stmt.go | 3 + pkg/sql/tokenizer/hash_comment_test.go | 122 ++++++++++ pkg/sql/tokenizer/lexical_extensions_test.go | 3 +- .../testdata/tokenizer_literal_golden.json | 217 ++++++++++++------ pkg/sql/tokenizer/tokenizer.go | 21 +- pkg/sql/tokenizer/unicode_escape.go | 2 +- .../realworld/mariadb/91_hash_comments.sql | 9 + testdata/realworld/mysql/91_hash_comments.sql | 9 + 19 files changed, 661 insertions(+), 95 deletions(-) create mode 100644 pkg/gosqlx/hash_comments_test.go create mode 100644 pkg/sql/parser/utility_cursor_test.go create mode 100644 pkg/sql/tokenizer/hash_comment_test.go create mode 100644 testdata/realworld/mariadb/91_hash_comments.sql create mode 100644 testdata/realworld/mysql/91_hash_comments.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 2935c0517..dc05014d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 identifier delimiters through rendering. ### Changed +- **MySQL / MariaDB `#` line comments**: with the MySQL or MariaDB dialect, `#` + now starts a line comment, like `--`. The tokenizer records it in `Comments` + (re-emitted by the formatter), `SplitStatements` no longer splits on a `;` + inside it, and transpiling to another dialect drops it instead of emitting `#` + (PostgreSQL XOR). Previously `WHERE x = 1 # c` parsed `#` as an operator and + `SELECT a FROM t # c` failed with E2002. Other dialects keep their `#` + meaning (PostgreSQL `#`, `#>`, `#>>`, `#-`; SQL Server `#temp`; Oracle `emp#`). - Parsed ClickHouse SAMPLE sizes and offsets now use inspectable expression fields, which take rendering precedence over the retained legacy string fields. - ClickHouse SAMPLE accepts only decimal numbers (hex such as `0x10` is @@ -79,6 +86,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 in dialects without them. ### Fixed +- `GRANT ... TO [`, `TO [AppUser`, `TO CURRENT_USER(`, MariaDB + `IDENTIFIED VIA ... USING PASSWORD(` and `COPY (` with an unclosed group no + longer panic with `index out of range [-1]`; they return an E2002 error + (expected `]` or `)`) in every dialect. - Watch now respects `--dialect`, re-validates changes and formats files in place with the CLI safety checks; MCP parsing accepts a dialect and documents comment preservation. - Correct transpile dialect help, avoid duplicate lint syntax errors, and rewrite diff --git a/internal/dropcheck/dropcheck.go b/internal/dropcheck/dropcheck.go index abfa92009..e7373d57b 100644 --- a/internal/dropcheck/dropcheck.go +++ b/internal/dropcheck/dropcheck.go @@ -79,6 +79,7 @@ var ignorable = map[string]bool{"AS": true, "ASC": true, "INNER": true, "OUTER": // Words returns the upper-cased identifier-like words of s, skipping // comments and quoted string literals but keeping quoted identifiers' text. +// For MySQL and MariaDB, '#' also starts a line comment (outside backticks). // The optional dialect uses the tokenizer's string escape rules; omitted means // generic SQL. PostgreSQL E-prefixed strings always allow backslash escapes. func Words(s string, dialect ...string) []string { @@ -87,11 +88,21 @@ func Words(s string, dialect ...string) []string { d = sqldialect.Parse(dialect[0]) } backslash := d.StringEscape() != sqldialect.StringEscapeNone + mysqlFamily := d == sqldialect.MySQL || d == sqldialect.MariaDB var words []string for i := 0; i < len(s); { c := s[i] switch { - case c == '-' && i+1 < len(s) && s[i+1] == '-': + case c == '`' && mysqlFamily: + // A backtick identifier: keep its words, but a '#' inside it does + // not start a comment. + end := strings.IndexByte(s[i+1:], '`') + if end < 0 { + end = len(s) - i - 1 + } + words = append(words, Words(s[i+1:i+1+end])...) + i += end + 2 + case c == '-' && i+1 < len(s) && s[i+1] == '-', c == '#' && mysqlFamily: for i < len(s) && s[i] != '\n' { i++ } diff --git a/internal/dropcheck/dropcheck_test.go b/internal/dropcheck/dropcheck_test.go index 8522bd057..6725188cb 100644 --- a/internal/dropcheck/dropcheck_test.go +++ b/internal/dropcheck/dropcheck_test.go @@ -103,3 +103,31 @@ func TestDroppedWordsCorpusCases(t *testing.T) { }) } } + +func TestWordsHashComments(t *testing.T) { + tests := []struct { + name string + dialect string + in string + want []string + }{ + {"mysql hash comment skipped", "mysql", "SELECT a # gone\nFROM t", []string{"SELECT", "A", "FROM", "T"}}, + {"mariadb hash comment skipped", "mariadb", "SELECT a #gone", []string{"SELECT", "A"}}, + {"mysql backtick hash kept", "mysql", "SELECT `a#b` FROM t", []string{"SELECT", "A", "B", "FROM", "T"}}, + {"postgresql hash is an operator", "postgresql", "SELECT a # b", []string{"SELECT", "A", "B"}}, + {"generic hash is not a comment", "", "SELECT a # b", []string{"SELECT", "A", "B"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got []string + if tt.dialect == "" { + got = Words(tt.in) + } else { + got = Words(tt.in, tt.dialect) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Words(%q, %q) = %v, want %v", tt.in, tt.dialect, got, tt.want) + } + }) + } +} diff --git a/pkg/gosqlx/hash_comments_test.go b/pkg/gosqlx/hash_comments_test.go new file mode 100644 index 000000000..34677101d --- /dev/null +++ b/pkg/gosqlx/hash_comments_test.go @@ -0,0 +1,110 @@ +// 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 gosqlx + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestHashCommentsIntegration(t *testing.T) { + const source = "SELECT a FROM t WHERE x = 1 # c\nAND y = 2" + for _, d := range []keywords.SQLDialect{keywords.DialectMySQL, keywords.DialectMariaDB} { + t.Run(string(d), func(t *testing.T) { + tree, err := ParseWithDialect(source, d) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + if len(tree.Statements) != 1 { + t.Fatalf("statements = %d", len(tree.Statements)) + } + sel, ok := tree.Statements[0].(*ast.SelectStatement) + if !ok { + t.Fatalf("statement = %T", tree.Statements[0]) + } + and, ok := sel.Where.(*ast.BinaryExpression) + if !ok || and.Operator != "AND" { + t.Fatalf("WHERE = %#v", sel.Where) + } + for _, side := range []struct { + expr ast.Expression + want string + }{{and.Left, "x = 1"}, {and.Right, "y = 2"}} { + eq, ok := side.expr.(*ast.BinaryExpression) + if !ok || eq.Operator != "=" || eq.SQL() != side.want { + t.Fatalf("predicate = %#v, want %s", side.expr, side.want) + } + } + trailing, err := ParseWithDialect("SELECT a FROM t # trailing", d) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(trailing) + formatted, err := FormatWithDialect("SELECT a FROM t # trailing", FormatOptions{}, d) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(formatted, "# trailing") { + t.Fatalf("comment lost: %q", formatted) + } + reparsed, err := ParseWithDialect(formatted, d) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(reparsed) + if len(reparsed.Statements) != 1 || FormatAST(reparsed) != FormatAST(trailing) { + t.Fatalf("format changed statement: %q", formatted) + } + got, err := Transpile(source, d, keywords.DialectPostgreSQL) + if err != nil { + t.Fatal(err) + } + if got != "SELECT a FROM t WHERE x = 1 AND y = 2" { + t.Fatalf("transpiled = %q", got) + } + }) + } +} + +func TestSplitStatementsHashComments(t *testing.T) { + for _, d := range []string{"mysql", "mariadb", "postgresql", ""} { + t.Run(d, func(t *testing.T) { + got := SplitStatements("SELECT 1; # a;b\nSELECT 2", d) + want := 3 + if d == "mysql" || d == "mariadb" { + want = 2 + } + if len(got) != want || got[0] != "SELECT 1" || !strings.Contains(got[len(got)-1], "SELECT 2") { + t.Fatalf("split = %#v", got) + } + }) + } + for _, d := range []string{"mysql", "mariadb"} { + for _, source := range []string{ + "BEGIN # ignored; END\nSELECT 1; SELECT 2; END; SELECT 3", + "BEGIN # ignored\nNOT # ignored\nATOMIC SELECT 1; SELECT 2; END; SELECT 3", + "DELIMITER $$\nCREATE PROCEDURE p() BEGIN # ignored $$; END\nSELECT 1; END$$\nDELIMITER ;\nSELECT 2;", + "BEGIN SELECT 1; END # ignored\nIF; SELECT 2;", + } { + if got := SplitStatements(source, d); len(got) != 2 { + t.Errorf("%s split %q = %#v", d, source, got) + } + } + } +} diff --git a/pkg/gosqlx/splitter.go b/pkg/gosqlx/splitter.go index 3e764697a..94228bc67 100644 --- a/pkg/gosqlx/splitter.go +++ b/pkg/gosqlx/splitter.go @@ -87,6 +87,7 @@ type dialectProfile struct { supportsEStrings bool supportsBackslash bool supportsNestedBlockComments bool + hashComments bool // Procedural-script features (see splitter_proc.go). tsql bool // GO batches; IF/WHILE/LOOP are not delimited by END IF etc. @@ -108,7 +109,7 @@ func profileForDialect(dialect string) dialectProfile { supportsNestedBlockComments: true, } case "mysql", "mariadb": - return dialectProfile{supportsBackticks: true, delimiter: true} + return dialectProfile{supportsBackticks: true, delimiter: true, hashComments: true} case "clickhouse": return dialectProfile{supportsBackticks: true} case "sqlserver", "mssql", "tsql": @@ -286,6 +287,10 @@ func splitCore(src string, p dialectProfile) (out []string, open bool) { inLine = true i += 2 + case c == '#' && p.hashComments: + inLine = true + i++ + case c == '/' && i+1 < len(src) && src[i+1] == '*': blockCommentDepth = 1 i += 2 diff --git a/pkg/gosqlx/splitter_proc.go b/pkg/gosqlx/splitter_proc.go index b6ffaef52..8009c292c 100644 --- a/pkg/gosqlx/splitter_proc.go +++ b/pkg/gosqlx/splitter_proc.go @@ -54,6 +54,7 @@ type procState struct { on, tsql, oracle bool depth int declMask uint64 + hashComments bool // stmtStart is true while the next word begins a statement (after ';', // BEGIN, THEN, ELSE, LOOP, a label, …). IF/WHILE/FOR/REPEAT only open a // block there, which keeps "DROP TABLE IF EXISTS" and IF() calls out. @@ -66,6 +67,7 @@ type procState struct { func (s *procState) init(p dialectProfile) { s.on = !p.noBlocks + s.hashComments = p.hashComments s.tsql = p.tsql s.oracle = p.oracle s.stmtStart = true @@ -73,7 +75,7 @@ func (s *procState) init(p dialectProfile) { // reset discards any open block (a batch separator ends them all). func (s *procState) reset() { - *s = procState{on: s.on, tsql: s.tsql, oracle: s.oracle, stmtStart: true} + *s = procState{on: s.on, tsql: s.tsql, oracle: s.oracle, stmtStart: true, hashComments: s.hashComments} } func (s *procState) semicolon() { @@ -161,7 +163,7 @@ func (s *procState) word(src string, i, j int) int { s.stmtStart = true return j } - if beginNamesColumn(src, i) || !beginIsBlock(src, j) { + if beginNamesColumn(src, i) || !beginIsBlock(src, j, s.hashComments) { return j } s.push(false) @@ -169,7 +171,7 @@ func (s *procState) word(src string, i, j int) int { case kwEnd: s.pop() - w, _, end := peekWord(src, j) + w, _, end := peekWordWithHash(src, j, s.hashComments) var b [12]byte switch string(foldWord(&b, w)) { case "try", "catch": @@ -252,7 +254,7 @@ func (s *procState) word(src string, i, j int) int { case kwIsAs: if s.oracle && s.routineHdr { s.routineHdr = false - w, _, _ := peekWord(src, j) + w, _, _ := peekWordWithHash(src, j, s.hashComments) var b [12]byte switch string(foldWord(&b, w)) { case "language", "external": @@ -328,8 +330,8 @@ func foldWord(buf *[12]byte, w string) []byte { // beginIsBlock reports whether the BEGIN ending at j opens a block rather // than starting a transaction (BEGIN; BEGIN TRANSACTION; BEGIN IMMEDIATE …) // or being a column or alias that happens to be called begin. -func beginIsBlock(src string, j int) bool { - w, c, end := peekWord(src, j) +func beginIsBlock(src string, j int, hashComments bool) bool { + w, c, end := peekWordWithHash(src, j, hashComments) if w == "" { // BEGIN :NEW.x := ..., BEGIN <