Repository navigation
fix(test-runner): split $$ heredocs and escaped backticks in splitQueries() - #1097
Open
Tyagiquamar wants to merge 1 commit into
Open
Tyagiquamar wants to merge 1 commit into
Tyagiquamar wants to merge 1 commit into
Conversation
…ries() The statement splitter in the upstream SQL test harness only knew single quotes, double quotes, backticks, backslash escapes outside backticks, and the two SQL comment styles. Two constructs in the server lexer were missing: - Dollar quoting ($$...$$ and $tag$...$tag$). A quote inside the heredoc toggled the splitter's quote state, so every following statement was merged into one, and a `;` inside the heredoc could split it apart. - A backslash-escaped backtick inside a quoted identifier, which closed the identifier early and left it open for the rest of the file. Both are implemented the way the server lexer does it (see src/Parsers/Lexer.cpp): the opening delimiter is `$`, an optional tag of word characters, and `$` again, and the heredoc runs to the next occurrence of that same delimiter; an unterminated delimiter is not a heredoc. Quoted identifiers now honor backslash escapes like string literals. Statement counts on the upstream files from ClickHouse#1096 now match the official lexer: 03011_definitive_guide_to_cast 82 -> 91, 04038_ascii_cjk_tokenizer 28 -> 43, 04097_tuple_inside_nullable_subcolumn_descendants 15 -> 18, and the 02683/02685/02686/02687/03032 heredoc files split at the right boundary. A 150-file sample of tests/queries/0_stateless produces unchanged output.
Tyagiquamar
requested review from
ccoltx,
chernser,
kavirajk and
mshustov
as code owners
November 1, 2026 10:20
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the internal SQL statement-splitting changes.
No open issues 🎉
What changed in this PR
Fixes #1096 by aligning the upstream SQL test harness’s statement splitter with ClickHouse’s handling of dollar-quoted strings and escaped backticks. Changes are limited to the internal harness; published client APIs are unaffected.
Changes:
- Preserve
$$...$$and$tag$...$tag$heredocs without interpreting their contents as statement boundaries. - Recognize backslash escapes inside backtick-quoted identifiers.
- Add regression tests for heredocs, non-heredoc dollar sequences, and escaped backticks.
| File | Description |
|---|---|
tests/clickhouse-test-runner/src/split-queries.ts |
Adds heredoc detection and backtick escape handling. |
tests/clickhouse-test-runner/__tests__/split-queries.test.ts |
Adds six regression cases covering the corrected splitting behavior. |
Review effort: Balanced
Merged
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1096.
Summary
splitQueries()in the upstream SQL test harness only tracked',"and backticks, backslash escapes outside backticks, and the two comment styles. The server lexer also has dollar quoting and honors backslash escapes inside quoted identifiers, so two constructs were mis-split:$$ ... $$/$tag$ ... $tag$heredocs (7 upstream files). A'or"inside the heredoc toggled the splitter's quote state, so the remaining statements were merged into one, and a;inside the heredoc could split it. Example:03011_definitive_guide_to_castwas seen as 82 statements instead of 91,04038_ascii_cjk_tokenizeras 28 instead of 43.`a\`b`closed the identifier early, leaving it open and merging the rest of the file:04097_tuple_inside_nullable_subcolumn_descendantswas seen as 15 statements instead of 18.Root Cause
src/split-queries.tshand-scans the SQL with three quote flags and no notion of dollar quoting. It also only entered the escape handling for'and"(if ((inSingleQuote || inDoubleQuote) && ch === "\\")), while the server lexer'squotedString<>()treats',"and`identically. There is no bug in ClickHouse; this is purely the harness's statement splitter disagreeing withsrc/Parsers/Lexer.cppabout where a statement ends.Technical Solution
Kept the hand-written scanner (the issue's measurement does not justify adopting the lexer, and 17 upstream files use syntax the lexer rejects) and taught it the two missing rules, mirroring the lexer:
findHeredocEnd()implements the lexer's$branch: an opening$, an optional tag of word characters ([A-Za-z0-9_]), and$again; the heredoc then extends to the next occurrence of the same delimiter. An unterminated delimiter is not a heredoc, and its$stays an ordinary character. The region is copied verbatim, so quotes, comment markers and semicolons inside it cannot affect the split.quotedString<''>()`.No parser rewrite, no new dependency, no change to
test-hint.tsor the backends.Resulting statement counts on the files named in the issue now match the official lexer:
0301182 -> 91,0403828 -> 43,0409715 -> 18, and02683/02685/02686/02687/03032split at the correct boundary. A 150-file sample oftests/queries/0_statelessproduces byte-identical output before and after, so no currently allowlisted file changes behavior. Adding the affected tests toupstream-allowlist.txtis deliberately left out: pertests/clickhouse-test-runner/AGENTS.md, allowlist growth is validated separately through the harness against both CI server versions.Test Plan
New cases in
tests/clickhouse-test-runner/__tests__/split-queries.test.ts(heredoc with quotes/backslashes, tagged heredoc, statement boundaries after a heredoc, unterminated$$,$foonot being a heredoc, escaped backtick in a quoted identifier). All four heredoc/backtick cases fail onmainand pass with the fix.Commands run locally (Node 24.22.1, Windows):
Statement counts old vs new were compared by running both implementations over the upstream files from the issue plus a 150-file sample of
tests/queries/0_stateless(ClickHouse master).No changelog entry:
tests/clickhouse-test-runneris a private, unpublished workspace package with noCHANGELOG.md, and no published package changes.Checklist