[ajitpratap0]

Summary

• Panic fix (HIGH-1): ParseWithDialect("GRANT SELECT ON t TO [", d) (also TO [AppUser and TO CURRENT_USER() panicked with index out of range [-1] for pg, mysql, mariadb, oracle, snowflake and clickhouse. The utility-grammar cursor stored the -1 sentinel from bracketEnd/groupEnd in c.i and then called c.fail, which runs seek(-1). Fixed at every such site: grantee (both cases), mariadbAuth (IDENTIFIED VIA ... USING PASSWORD() and copyEnd (COPY (). As a second guard, Parser.seek now treats a negative index as EOF. These inputs now return a structured E2002 error ("expected ]" or "expected )").

• MySQL/MariaDB # comments (HIGH-2): with the MySQL or MariaDB dialect, # now starts a line comment, the same as --:

  • the tokenizer records it in Comments (Style LineComment, verbatim text), and the formatter re-emits it;
  • SplitStatements and its procedural BEGIN/END lookahead skip it, so a ; inside the comment no longer splits;
  • highlight error recovery and internal/dropcheck (used by the realworld corpus test) know about it too;
  • transpiling to PostgreSQL drops the comment instead of emitting #, which PostgreSQL reads as XOR.

  Other dialects keep their current meaning of #: PostgreSQL #, #>, #>> and #-; SQL Server #temp and ##global; Oracle emp#; generic.

• Tests: a table test over all 8 dialects that recovers any panic, a seek(-1) unit test, new FuzzParse seeds, tokenizer/splitter/format/transpile/dropcheck cases, and testdata/realworld/{mysql,mariadb}/91_hash_comments.sql. I regenerated the tokenizer literal golden. Among existing entries, only the mysql/mariadb digests changed, and only for inputs that contain #. The other changes are new entries and line-number shifts.

Test Plan

• task check (fmt, vet, lint, test:race): passes, no FAIL or panic in the output
• task test:cbinding: passes
• Targeted go test -race on tokenizer, parser, gosqlx (including TestRealWorldCorpus and splitter/parser agreement), formatter, transpiler, highlight and internal/...
• Tokenizer and parser benchmarks against origin/main, interleaved, -count=6, compared with benchstat: no significant change. The largest difference was +1.39% on Tokenizer/ComplexSQL. B/op and allocs/op are identical.

Breaking Changes

Behaviour change for MySQL/MariaDB only: # used to tokenize as TokenTypeSharp (an operator) and now starts a comment. This matches what MySQL and MariaDB do. No API changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 28, 2026 7:45pm UTC

[github-actions]

Claude finished @ajitpratap0's task in 23s —— View job

---

Review of PR #881: GRANT cursor panic fix and MySQL/MariaDB # comments

I reviewed the diff by reading it only. I did not run the tests or benchmarks, so the results below rely on the PR description.

Verdict: LGTM, with a few small nits. Nothing blocking.

Panic fix (HIGH-1)

• The root cause is correctly identified. c.i = bracketEnd(...) stored -1, and c.fail then ran seek(-1). Computing end in a local and assigning c.i only on success fixes all four sites: grantee (2), mariadbAuth and copyEnd.
• copyEnd also drops the old c.i = start + 1 reset. That is correct now that c.i is never clobbered.
• In mariadbAuth, the new typeAt(c.i) != LParen check gives a sensible "string" error when PASSWORD has no (. Before, groupEnd was called on a non-paren token. This is a good tightening.
• Parser.seek treating i < 0 as EOF (utility_stmt.go) is a sound second guard. One trade-off: it can hide a future sentinel-leak bug by turning a panic into a vaguer EOF error. The seek(-1) unit test and the all-dialect panic-recovery table test should catch regressions, so I'm fine with it.

# comments (HIGH-2)

• Tokenizer: skipLineComment(openerLen) is a clean generalisation. The dialect gate (MySQL and MariaDB only) leaves PG #>, #>> and #-, SQL Server #temp and Oracle emp# untouched.
• Nit 1, unicode_escape.go skipSpaceAndComments: it handles only -- and /* */, so # isn't treated as a comment there. That path is for U&'..' UESCAPE, which isn't a MySQL feature, so it is probably fine. A short comment saying so would stop someone later "fixing" it.
• Nit 2, splitter_proc.go: peekWord is now just a wrapper around peekWordWithHash(..., false). If it has no remaining callers, remove it. Otherwise, add a doc comment to peekWordWithHash, which has none.
• Nit 3, MySQL's -- rule: MySQL requires whitespace or a control character after --, while # has no such rule. This behaviour already existed and isn't in scope here. The same-as--- wording in the PR is slightly loose for that reason.
• Nit 4, dropcheck backtick handling: Words(s[i+1:i+1+end]) recurses with no dialect, so the inner text is scanned as generic SQL. That is fine for identifiers. It also means a ''-style escape inside backticks is treated as generic, which is harmless. An unterminated backtick consumes the rest of the input, which matches how the tokenizer behaves.
• The splitter and highlight changes are consistent with the tokenizer. The highlighter's lastUnclosed correctly scopes # to MySQL and MariaDB.
• Transpiling to PostgreSQL by dropping the comment is the right call, since # is XOR there.

Performance and security

• The added tokenizer branch is a single rune and dialect comparison on a path that already switches on r. Your benchstat numbers (at most +1.39%, identical allocs) are consistent with that.
• The # rule makes the splitter stricter, which improves injection-adjacent correctness. A ; inside a # comment no longer splits statements. If security scanning does its own comment stripping, it may be worth checking that it agrees. I didn't look at that.

Tests

• Coverage is good: the all-dialect panic table, seek(-1), fuzz seeds, tokenizer, splitter, formatter, transpiler, dropcheck and highlight cases, and the realworld corpus files.
• The golden file changed in the expected places only. The regenerated digests and the line-number shifts are reasonable to review as described.
• Optional extra cases: a # inside a MySQL string or backtick identifier within the splitter test, and # at EOF with no trailing newline. Some of these may already be present in hash_comments_test.go.

The CHANGELOG.md entry and the breaking-change note for MySQL and MariaDB are appropriate.

[github-actions]

Claude finished @ajitpratap0's task in 31s —— View job

---

Review of PR #881: GRANT cursor panic fix and MySQL/MariaDB # comments

I read the diff for the parser, tokenizer, splitter and dropcheck changes. I did not run the tests.

Verdict: good fix. There is one small gap to close (item 1) and a few nits.

Panic fix

• The cursor fixes are correct. Each site computes end locally and assigns c.i only on success, so c.fail never sees the -1 sentinel. This covers grantee (2 sites), mariadbAuth and copyEnd.
• The new typeAt(c.i) != LParen check in mariadbAuth is a good tightening.
• The seek(i < 0) guard is a sensible second layer. It can turn a future sentinel leak into a vaguer EOF error, but your all-dialect panic table test covers that.

# comments

1. Gap: commentsOnly ignores # (pkg/gosqlx/splitter_proc.go:441).
   • commentsOnly and elseFollows still call peekWord, which hard-codes hashComments=false.
   • ParseReader (pkg/gosqlx/reader.go:140) uses commentsOnly to skip comment-only segments. In MySQL/MariaDB, a trailing segment such as SELECT 1; # done is not recognised as comment-only. It is passed to ParseTree instead of being skipped.
   • The segment then fails to parse or yields an empty tree, depending on how the parser handles an empty statement.
   • Suggested fix: give commentsOnly a hashComments parameter and have the caller pass dialectProfile.hashComments (or the dialect). A reader-level test with a trailing # comment would pin it down.
   • elseFollows is only used when p.tsql is set, so it is safe as is.
2. peekWord and peekWordWithHash (pkg/gosqlx/splitter_proc.go:397-401).
   • After item 1, peekWord has only the elseFollows call left, plus commentsOnly if it stays unchanged.
   • Consider folding the two into one function with the flag, or at least add a doc comment on peekWordWithHash.
3. unicode_escape.go skipSpaceAndComments. It does not treat # as a comment. This is fine for U&'..' UESCAPE, which isn't a MySQL feature. A one-line comment saying so would stop a later "fix".
4. MySQL -- rule. MySQL requires whitespace after --, while # has no such rule. That behaviour is outside this PR, but "same as --" in the description is slightly loose.
5. dropcheck backtick handling. The recursive Words(...) call has no dialect. That is harmless for identifiers, and an unterminated backtick consumes the rest of the input, which matches the tokenizer.

Performance and security

• The tokenizer change adds one rune and dialect comparison on an existing switch. The benchstat results you posted are consistent with that.
• The splitter no longer splits on a ; inside a # comment for MySQL/MariaDB. That is more correct and safer.
• If pkg/sql/security does its own comment stripping, check that it treats # the same way for MySQL/MariaDB. I did not look at it.

Tests

• Coverage is thorough: the all-dialect panic table, seek(-1), fuzz seeds, tokenizer, splitter, formatter, transpiler, dropcheck and highlight cases, and the realworld corpus files.
• Optional extra cases: # at EOF with no trailing newline, # inside a MySQL string or backtick identifier in the splitter, and the ParseReader trailing-comment case from item 1.

The CHANGELOG entry and breaking-change note are appropriate.