Fix jdbc-v2: lex heredoc strings as a single literal in the JavaCC parser - #3030
Open
polyglotAI-bot wants to merge 3 commits into
Open
Fix jdbc-v2: lex heredoc strings as a single literal in the JavaCC parser#3030polyglotAI-bot wants to merge 3 commits into
polyglotAI-bot wants to merge 3 commits into
Conversation
…rser The JavaCC grammar had no heredoc token, so the body of `$$...$$` / `$tag$...$tag$` was lexed as ordinary SQL. A body character with no standalone token (`!`, `&`, `|`, `~`) raised a TokenMgrException, which is not a ParseException and so escaped dataClause()'s recovery, leaving the whole statement classified as UNKNOWN: an INSERT was reported as a result-set-bearing statement with no table name and no values-list positions. Fixes: #3029
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
This was referenced Aug 4, 2026
…c-heredoc-literal # Conflicts: # jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b2dc45b. Configure here.
A '$' inside a heredoc body is data, not a tag delimiter: the server reads $$a$b$$ as a$b. Add rows to the shared data provider so all three parser backends pin that such a statement keeps its classification and values-list positions.
Collaborator
Author
SonarCloud "Quality Gate failed" on this PR is not caused by this diffThe gate fails on one condition only — Coverage on New Code 75.0% (< 80%). All other conditions pass (reliability/security/maintainability A, duplication 0.0%, hotspots 100%). The lines Sonar counts as "new code" here do not belong to this PR:
That is 20 of 20 new lines to cover, all in
No source line added by this PR is uncovered, so there is no coverage change we can make in this diff — adding tests for unrelated |
|
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
2 participants
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.
Description
Fixes #3029.
The
jdbc-v2JavaCC grammar (the default parser,jdbc_sql_parser=JAVACC) has no heredoc token, so the body of a$$...$$/$tag$...$tag$literal is lexed as ordinary SQL —$$aeven matchesIDENTIFIER. A body character that is not a valid standalone token (!,&,|,~) therefore raises aTokenMgrException, which is not aParseException, so the recovery indataClause()never sees it. It escapes toClickHouseSqlParser.parse, whosecatch (Exception)discards the parse result and leaves the pre-initializedStatementType.UNKNOWNstatement: anINSERTis reported asisInsert() == false,isHasResultSet() == true,hasErrors() == true, with no table name, no values-list positions and no value groups — which silently disables the batch values template, the table-name based paths (e.g. the beta RowBinary writer) and theexecute()/getMetaData()routing, and logs a parse warning. The server accepts all of these statements, and both ANTLR4 backends already classify them correctly.The fix teaches the lexer about heredocs (the grammar's own
FIXMEindataClause()flagged the missing lexical state) so a heredoc is a single string literal, as it is on the server.Changes
jdbc-v2/src/main/javacc/ClickHouseSqlParser.jjHEREDOC_LITERALtoken:$tag$body$tag$with an optional[a-zA-Z0-9_]tag. It is declared beforeIDENTIFIERso that on an equal-length match (e.g.$$a$$, whichIDENTIFIERalso matches because$is in both its first and continuation sets) the heredoc wins.$and the opening/closing tags are not required to be equal. An unterminated tag ($foo$bar) stays the longerIDENTIFIERmatch, i.e. existing behavior is preserved — and that is also how the server reads it (SELECT $foo$barfails withUNKNOWN_IDENTIFIER '$foo$bar', not an unterminated-heredoc error).literal()accepts the new token, which makes a heredoc usable wherever a string literal is a value — this reaches bothcolumnExpr()andanyColumnExpr(), so VALUES lists, select lists and the "not interested" statements are all covered.CHANGELOG.md: entry under0.11.0-rc1→ Bug Fixes.No public API change; no configuration change. The change is additive at the lexer level: input that parsed before still parses, and input that previously raised a lexer error is now accepted.
Test
jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java— the shared parser-facade suite, so the cases run against all three backends (JAVACC,ANTLR4,ANTLR4_PARAMS_PARSER):testHeredocStatements(@DataProvider) assertshasErrors,isInsert,isHasResultSet, the extracted table and the exact values-list substring (sliced from the caller's SQL via the reported start/stop positions) for: each offending body character (!,&,|,~,@), the tagged form, a body with whitespace, parentheses and commas in a body (so a shifted values list would be caught), two heredocs in one values list, and a heredoc in aSELECTlist.SELECT $foo$bar FROM t) and a$inside an identifier (SELECT a$b FROM t) keep parsing as identifiers, and a quoted'a!b'literal keeps its existing handling.testHeredocStatementsJavaCcOnlycovers two bodies the ANTLR4 grammars do not accept yet (a;inside the body — which must not split the statement — and an empty$$$$body), guarded with the file's existingjavaCcBackendpattern.Verification: the new cases fail on
main(9 failures for the JavaCC backend) and pass with the fix; the fulljdbc-v2unit suite is green (1343 tests), including the ANTLR4 backends, with no existing test modified.Note on scope: the character-scanning placeholder pass (
parseParameters) is separately not heredoc-aware — that is #3009 / PR #3010 and is deliberately untouched here. The legacyclickhouse-jdbc(v1) module carries a copy of this grammar with the same missing token; it is not changed in this PR.Pre-PR validation gate
main@ 1a11756, passes with the fix)jdbc-v2unit suite green)AGENTS.md(single logical change, targeted Maven runs,@DataProviderinstead of near-identical methods, no issue numbers inside test code, CHANGELOG updated)docs/features.mdchange (no feature added, removed or intentionally changed)