[ajitpratap0]

Summary

Follow-ups from the pre-merge review of #866–#868: SplitStatements and the parser now agree on statement boundaries. The audit harness and drop detector are also tightened.

• Backslash escapes: the splitter applies each dialect's real string-escape rule. MySQL, MariaDB and ClickHouse use backslash escapes, as do PostgreSQL E'' strings. SELECT 'a\';b'; SELECT 3 is now 2 statements in MySQL, not 3 fragments.
• Parentheses: a ; inside (...) no longer ends a statement, e.g. PostgreSQL CREATE RULE ... DO ALSO (a; b). Unbalanced nesting falls back to one extra linear pass, so a malformed fragment can't swallow the rest of a script.
• GO lines: GO, GO 2, GO -- c, GO 2 /* c */ and multi-line block comments after GO are handled by one helper (internal/batch.TextEnd), used by both the splitter and the parser.
• Oracle /: a line holding only / is a SQL*Plus execute command in the parser as well as the splitter. A / that shares its line with SQL or a comment stays division.
• dropcheck: words are compared by count, so one dropped AND among several is caught. Backslash handling is per dialect.
• Harness: scripts/dialect-audit and TestRealWorldCorpus share one pipeline (internal/dialectaudit). Statements holding only block comments count as empty. -dialect is validated. Glob errors and empty dialect directories now fail the test. format_corpus_test maps /sqlserver/ paths.

Test Plan

• gofmt, go vet, golangci-lint run (0 issues), task test:race, task test:cbinding
• New tests: splitter_boundaries_test.go (escapes, parentheses, Oracle / cases, formatting Oracle END; + / scripts), splitter_batch_comments_test.go (every GO variant, including CRLF), internal/batch/text_test.go, internal/dialectaudit, scripts/dialect-audit/main_test.go, and the multiset dropcheck tests
• TestRealWorldCorpus green; known_failures stays empty with the multiset check

Breaking Changes

None to the Go API. Behaviour changes:

• Oracle input with a lone / line between expressions now parses as two statements, not a division.
• SplitStatements output changes for the escape and parenthesis cases above.

Known gap: FormatAST output drops the SQL*Plus / lines, as it did before. Keeping them, the way GO lines are kept, is a follow-up.

🤖 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 27, 2026 8:32am UTC

[github-actions]

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

---

Review of PR #869: splitter/parser boundary agreement

I read the diff only and did not run the tests or the linter. The design is sound. Sharing batch.TextEnd between the splitter and the parser is the right way to stop the two from drifting apart. The new tests cover the edge cases well, including CRLF and a cross-check of TextEnd against batch.End on tokenized input. I found no blocking bugs. These are the things I'd look at:

Should fix or consider

1. One stray ( turns off BEGIN…END tracking for the whole script (pkg/gosqlx/splitter.go, splitWithProfile).

   • The fallback sets both noBlocks and noParens whenever open is true.
   • So a script with one unbalanced paren, such as a typo in one statement, also loses block handling for every later CREATE PROCEDURE … BEGIN a; b; END. Those procedures get shredded at each ;.
   • Before this PR, only unbalanced blocks triggered that fallback.
   • Consider retrying in stages. First drop only parens if st.depth == 0, and drop blocks only if blocks are the unbalanced part. That means splitCore should return which of the two was left open.
   • A test for "unbalanced ( followed by a balanced procedure body" would pin this down.

2. The Oracle lone-/ change affects multi-line division.

   • SELECT a\n/\nb FROM t is now two statements. The PR notes this, and it matches SQL*Plus.
   • The token-only path (preprocess → oracleExecuteLines("")) can't see the source text. There, any / alone on its token line becomes ;, even if the source line has other content such as a comment-only neighbour.
   • The doc comment mentions this limit. Please add a test that pins the token-only behaviour so it can't change by accident.
   • parseWithSource re-implements the preprocess step for Oracle instead of calling ParseFromModelTokens. If ParseFromModelTokens does any other setup, such as resets or hooks, the two paths will diverge. Please confirm they're equivalent, or factor out the shared part.

3. Backslash escapes are tied to dialect only (SplitStatements).

   • MySQL with NO_BACKSLASH_ESCAPES, or the same text in ANSI mode, will mis-split 'a\'. That is probably acceptable. A line in the doc comment would make the limit explicit.
   • Backslash escapes inside double-quoted strings (inDouble) are not handled. MySQL honours them, so "a\";b" still splits wrongly. Please confirm that is intentional.

Nits

• batch.TextEnd (internal/batch/text.go): the depth variable never exceeds 1, because block comments don't nest. It can become a closed bool. The same applies to dialectaudit.Empty.
• internal/dropcheck/dropcheck.go: c == '\'' || (c == 'E' || c == 'e') && … is correct, but explicit parentheses around the && group would help readers. go vet and some linters flag mixed ||/&&.
• The multiset comparison in DroppedWords can give false positives when the formatter legitimately normalises a word's multiplicity. An example is a keyword it repeats or elides. The corpus is green, so this is theoretical. known_failures staying empty is a good guard.
• Compact truncates by runes, which is fine. The Stage strings are bare literals shared between the package and its callers. Constants would prevent typos.

Performance and security

• Both splitter passes are linear. The two-pass bound is documented and correct.
• oracleExecuteLines scans source lines once through a forward-only offset. It copies each token struct (t := p.tokenBuf[i]), which is a small cost.
• I saw no security concerns. Splitter changes make it less likely that a ; hidden in a string or escape is mis-split, which helps downstream validation.

Test coverage is strong. Gaps I'd add: item 1 above, E'..\'..' in PostgreSQL combined with parens, and a test that GO inside an open paren still splits as a directive (or doesn't, whichever is intended).
· branch fix/wp-e