From 34792872c4ec7fc170c6670054dd618ca9554b29 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Wed, 28 Oct 2026 17:45:41 +0530 Subject: [PATCH] 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 2935c051..dc05014d 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 abfa9200..e7373d57 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 8522bd05..6725188c 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 00000000..34677101 --- /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 3e764697..94228bc6 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 b6ffaef5..8009c292 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 <