From b86a340ce1afb4fd19f4ea6862b4eb4b064e4dd4 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Tue, 4 Aug 2026 06:17:13 +0000 Subject: [PATCH 1/3] Fix jdbc-v2: lex heredoc strings as a single literal in the JavaCC parser 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: https://github.com/ClickHouse/clickhouse-java/issues/3029 --- CHANGELOG.md | 6 ++ .../src/main/javacc/ClickHouseSqlParser.jj | 10 +++ .../internal/BaseSqlParserFacadeTest.java | 68 +++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c92a57a..88fc5c218 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,12 @@ - **[client-v2]** Fixed LZ4 input streams not closing their underlying HTTP response stream. Closing an LZ4 stream returned by `QueryResponse.getInputStream()` now releases the wrapped transport stream, including after a partial read. (https://github.com/ClickHouse/clickhouse-java/issues/2985) +- **[jdbc-v2]** Fixed the default JavaCC SQL parser aborting on a heredoc string (`$$body$$`, `$tag$body$tag$`) + whose body contains a character that is not a valid SQL token on its own, such as `!`, `&`, `|` or `~`. The + lexer had no heredoc token, so such a body raised a lexer error that left the statement classified as + `UNKNOWN` — an INSERT was reported as a result-set-bearing statement with no table name and no values-list + positions, which disables the batch values template and the table-name based paths. A heredoc is now lexed + as a single string literal. (https://github.com/ClickHouse/clickhouse-java/issues/3029) - **[client-v2, jdbc-v2]** Reduced noisy and potentially sensitive logging; SQL that fails to parse is no longer logged at `WARN` (it could contain credentials/PII). (https://github.com/ClickHouse/clickhouse-java/issues/2970) - **[client-v2]** Fixed `BigDecimal` values written into a `Dynamic` column being silently truncated when the diff --git a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj index d0c088615..7456f863f 100644 --- a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj +++ b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj @@ -1007,6 +1007,7 @@ Token literal(): { Token t = null; } { t = dateLiteral() | t = numberLiteral() | t = + | t = | t = ) { return t; } @@ -1296,6 +1297,15 @@ TOKEN: { ( ~[] | ~["'", "\\"] | "''")* > } +// heredoc string literal: $$body$$ or $tag$body$tag$ +// Matched loosely, like the rest of this grammar: the opening and closing tags are not required to +// be equal and a body cannot contain '$'. An unterminated tag (e.g. `$foo$bar`) does not match and +// keeps being lexed as an identifier, which is also how the server reads it. +TOKEN: { + (~["$"])* > + | <#HEREDOC_TAG: ( | | )* > +} + TOKEN: { | | | ) ( | | | )* diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java index 945701ad0..0355b60c2 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java @@ -257,6 +257,74 @@ private void testCase(String sql, String expectedTableName) { Assert.assertEquals(stmt.getTable(), expectedTableName, "Table name mismatch for: " + sql); } + @Test(dataProvider = "heredocStatementsDP") + public void testHeredocStatements(String sql, boolean insert, String expectedTable, String expectedValuesList) { + ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); + Assert.assertFalse(stmt.isHasErrors(), "Query should parse without errors: " + sql); + Assert.assertEquals(stmt.isInsert(), insert, "Insert type mismatch for: " + sql); + Assert.assertEquals(stmt.isHasResultSet(), !insert, "Result set flag mismatch for: " + sql); + Assert.assertEquals(stmt.getTable(), expectedTable, "Table name mismatch for: " + sql); + if (expectedValuesList == null) { + Assert.assertEquals(stmt.getAssignValuesListStartPosition(), -1, "Should have no values list: " + sql); + } else { + Assert.assertEquals(sql.substring(stmt.getAssignValuesListStartPosition(), + stmt.getAssignValuesListStopPosition() + 1), expectedValuesList, + "Values list mismatch for: " + sql); + } + } + + @DataProvider + public static Object[][] heredocStatementsDP() { + return new Object[][] { + // A heredoc body is opaque: characters that are not valid SQL tokens on their own + // must not break the statement classification + {"INSERT INTO t VALUES ($$a!b$$, 1)", true, "t", "($$a!b$$, 1)"}, + {"INSERT INTO t VALUES ($$a&b$$, 1)", true, "t", "($$a&b$$, 1)"}, + {"INSERT INTO t VALUES ($$a|b$$, 1)", true, "t", "($$a|b$$, 1)"}, + {"INSERT INTO t VALUES ($$a~b$$, 1)", true, "t", "($$a~b$$, 1)"}, + {"INSERT INTO t VALUES ($$a@b$$, 1)", true, "t", "($$a@b$$, 1)"}, + // Tagged form and a body with whitespace + {"INSERT INTO t (c1, c2) VALUES ($tag_1$a!b$tag_1$, 1)", true, "t", "($tag_1$a!b$tag_1$, 1)"}, + {"INSERT INTO t VALUES ($$a b$$, 1)", true, "t", "($$a b$$, 1)"}, + // Parentheses and commas in a body must not shift the values list positions + {"INSERT INTO t VALUES ($$a(b,c)$$, 1)", true, "t", "($$a(b,c)$$, 1)"}, + // Two heredocs in one values list are two separate literals + {"INSERT INTO t VALUES ($$a!b$$, $$c!d$$)", true, "t", "($$a!b$$, $$c!d$$)"}, + // A heredoc is a value expression anywhere a string literal is accepted + {"SELECT $$a!b$$ AS x FROM t", false, "t", null}, + // Contrast: an unterminated tag is not a heredoc and stays an identifier + {"SELECT $foo$bar FROM t", false, "t", null}, + {"SELECT a$b FROM t", false, "t", null}, + // Contrast: a quoted string literal keeps its existing handling + {"INSERT INTO t VALUES ('a!b', 1)", true, "t", "('a!b', 1)"}, + }; + } + + @Test(dataProvider = "javaCcHeredocStatementsDP") + public void testHeredocStatementsJavaCcOnly(String sql, String expectedValuesList) { + // The ANTLR4 grammars do not accept these two heredoc bodies yet, so the expectations only + // hold for the JavaCC backend. + if (!javaCcBackend) { + return; + } + ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); + Assert.assertFalse(stmt.isHasErrors(), "Query should parse without errors: " + sql); + Assert.assertTrue(stmt.isInsert(), "Should be an INSERT: " + sql); + Assert.assertEquals(sql.substring(stmt.getAssignValuesListStartPosition(), + stmt.getAssignValuesListStopPosition() + 1), expectedValuesList, + "Values list mismatch for: " + sql); + } + + @DataProvider + public static Object[][] javaCcHeredocStatementsDP() { + return new Object[][] { + // A statement separator inside a heredoc body must not split the statement + {"INSERT INTO t VALUES ($$a;b$$, 1)", "($$a;b$$, 1)"}, + // Empty body + {"INSERT INTO t VALUES ($$$$, 1)", "($$$$, 1)"}, + }; + } + @Test public void testInsertColumnNamesAreUnescaped() { /* From d4084c95d824cb6405612a82152088f2f9b135f7 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:37:02 +0000 Subject: [PATCH 2/3] Pin heredoc bodies containing a single dollar sign in the parser tests 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. --- .../clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java index 417fe80c5..bd1eac2d6 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java @@ -377,6 +377,11 @@ public static Object[][] heredocStatementsDP() { // Tagged form and a body with whitespace {"INSERT INTO t (c1, c2) VALUES ($tag_1$a!b$tag_1$, 1)", true, "t", "($tag_1$a!b$tag_1$, 1)"}, {"INSERT INTO t VALUES ($$a b$$, 1)", true, "t", "($$a b$$, 1)"}, + // A single '$' in the body is data, not a tag delimiter (the server reads + // $$a$b$$ as a$b), so the statement must keep its classification + {"INSERT INTO t VALUES ($$a$b$$, 1)", true, "t", "($$a$b$$, 1)"}, + {"INSERT INTO t VALUES ($tag$a$b$tag$, 1)", true, "t", "($tag$a$b$tag$, 1)"}, + {"SELECT $$a$b$$ AS x FROM t", false, "t", null}, // Parentheses and commas in a body must not shift the values list positions {"INSERT INTO t VALUES ($$a(b,c)$$, 1)", true, "t", "($$a(b,c)$$, 1)"}, // Two heredocs in one values list are two separate literals From cc247d226b7a09e8d092e57b15b575b8f6beda58 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:00:36 +0000 Subject: [PATCH 3/3] Cover multiline and invalid heredoc strings in the parser tests Adds the review-requested cases and fixes the one real defect they found. Multiline: a heredoc is the only ClickHouse string that can hold raw line breaks, so 8 rows pin bodies with LF and CRLF, a body with a line break and a character that has no standalone token, a multiline statement, and a comment opener inside a body. Verified against the server: `SELECT $$line1 line2$$` returns the whole body as one value. Invalid: 11 rows pin unterminated heredocs, mismatched tags, an unpaired dollar, a tag with whitespace, two adjacent heredocs and a statement cut off inside the values list. The backends classify these differently, so the test pins the shared contract: the parser returns a statement instead of throwing, and the values-list positions either address the SQL or stay unset. Three of those rows failed: JavaCC records the values-list start position and then fails before the closing parenthesis, so the end position is never set and `parsePreparedStatement` threw a `NullPointerException` unboxing it. Both positions now stay unset when only one is known. --- CHANGELOG.md | 5 +- .../jdbc/internal/SqlParserFacade.java | 8 ++- .../internal/BaseSqlParserFacadeTest.java | 51 +++++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ddbdfefb..9605a72f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,7 +145,10 @@ lexer had no heredoc token, so such a body raised a lexer error that left the statement classified as `UNKNOWN` — an INSERT was reported as a result-set-bearing statement with no table name and no values-list positions, which disables the batch values template and the table-name based paths. A heredoc is now lexed - as a single string literal. (https://github.com/ClickHouse/clickhouse-java/issues/3029) + as a single string literal. Malformed SQL that opens a values list but never closes it — for example an + invalid heredoc such as `INSERT INTO t VALUES ($$a!b$$` — also no longer makes `parsePreparedStatement` + throw a `NullPointerException`: the values-list positions stay unset when only the start position is + known. (https://github.com/ClickHouse/clickhouse-java/issues/3029) - **[client-v2, jdbc-v2]** Reduced noisy and potentially sensitive logging; SQL that fails to parse is no longer logged at `WARN` (it could contain credentials/PII). (https://github.com/ClickHouse/clickhouse-java/issues/2970) - **[client-v2]** Fixed `BigDecimal` values written into a `Dynamic` column being silently truncated when the diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java index c2d0202af..bf61574b5 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java @@ -100,8 +100,12 @@ public ParsedPreparedStatement parsePreparedStatement(String sql) { stmt.setAssignValuesGroups(parsedStmt.getValueGroups()); Integer startIndex = parsedStmt.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_START); - if (startIndex != null) { - int endIndex = parsedStmt.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_END); + Integer stopIndex = parsedStmt.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_END); + // Malformed SQL can leave a values list open: the start position is recorded while the + // parser fails before the closing parenthesis, so the end position is never set. Leave both + // positions unset in that case instead of failing on the missing one. + if (startIndex != null && stopIndex != null) { + int endIndex = stopIndex; stmt.setAssignValuesListStartPosition(startIndex); stmt.setAssignValuesListStopPosition(endIndex); String query = parsedStmt.getSQL(); diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java index bd1eac2d6..1894ef8df 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java @@ -393,6 +393,57 @@ public static Object[][] heredocStatementsDP() { {"SELECT a$b FROM t", false, "t", null}, // Contrast: a quoted string literal keeps its existing handling {"INSERT INTO t VALUES ('a!b', 1)", true, "t", "('a!b', 1)"}, + // Multiline bodies: a heredoc is the only ClickHouse string that can hold raw line + // breaks, so neither the line break nor what follows it may end the literal + {"INSERT INTO t VALUES ($$line1\nline2$$, 1)", true, "t", "($$line1\nline2$$, 1)"}, + {"INSERT INTO t VALUES ($tag$line1\nline2$tag$, 1)", true, "t", "($tag$line1\nline2$tag$, 1)"}, + {"INSERT INTO t VALUES ($$line1\r\nline2$$, 1)", true, "t", "($$line1\r\nline2$$, 1)"}, + {"INSERT INTO t VALUES ($$a!b\nc|d$$, 1)", true, "t", "($$a!b\nc|d$$, 1)"}, + {"INSERT INTO t\nVALUES\n($$a\nb$$,\n1)", true, "t", "($$a\nb$$,\n1)"}, + // A comment opener inside a multiline body is data, not a comment + {"INSERT INTO t VALUES ($$-- still data\nmore data$$, 1)", true, "t", + "($$-- still data\nmore data$$, 1)"}, + {"INSERT INTO t VALUES ($$/* still data\n*/$$, 1)", true, "t", "($$/* still data\n*/$$, 1)"}, + {"SELECT $$line1\nline2$$ AS x FROM t", false, "t", null}, + }; + } + + @Test(dataProvider = "malformedHeredocStatementsDP") + public void testMalformedHeredocStatementsAreHandledGracefully(String sql) { + // Invalid heredoc strings (unterminated, mismatched or empty tags) must not make the parser + // throw: the driver relies on the returned statement to decide how to run the query. The + // backends classify these differently, so only the shared contract is pinned here. + ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); + Assert.assertNotNull(stmt, "Parser should return a statement for: " + sql); + int start = stmt.getAssignValuesListStartPosition(); + int stop = stmt.getAssignValuesListStopPosition(); + if (start >= 0 || stop >= 0) { + Assert.assertTrue(start >= 0 && stop >= start && stop < sql.length(), + "Values list positions should address the SQL or stay unset, got start=" + start + + " stop=" + stop + " for: " + sql); + } + } + + @DataProvider + public static Object[][] malformedHeredocStatementsDP() { + return new Object[][] { + // Unterminated heredoc: the closing tag never arrives + {"INSERT INTO t VALUES ($$abc, 1)"}, + {"INSERT INTO t VALUES ($tag$abc, 1)"}, + {"SELECT $$abc"}, + {"SELECT $tag$"}, + {"SELECT $$"}, + // A single unpaired dollar cannot close a heredoc + {"INSERT INTO t VALUES ($$abc$, 1)"}, + // Mismatched opening and closing tags + {"INSERT INTO t VALUES ($tag$abc$other$, 1)"}, + // A tag cannot hold whitespace, so this is not a heredoc at all + {"INSERT INTO t VALUES ($ $a$ $, 1)"}, + // Two heredocs with no separator between them + {"INSERT INTO t VALUES ($$a!b$$$$c!d$$, 1)"}, + {"SELECT $$abc$$$$def"}, + // The statement is cut off inside the values list + {"INSERT INTO t VALUES ($$a!b$$"}, }; }