From 83a4952544990a62e643c025cfab3b5dea727062 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Tue, 4 Aug 2026 00:30:45 +0000 Subject: [PATCH 1/2] Fix jdbc-v2: NPE when the JavaCC parser cannot parse an INSERT VALUES list dataClause() records the values list's start position when it matches the opening parenthesis and its end position when it matches the closing one. Its ParseException recovery skips to the end of the statement, so a values list abandoned in between left the start position recorded without its matching end position, which parsePreparedStatement then unboxed unguarded. Drop both positions in the recovery block so consumers see a complete pair or none, and require both to be present before using them. With no values list positions the driver falls back to its generic parameter-substitution path, which handles these statements correctly. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/3013 --- CHANGELOG.md | 7 +++ .../jdbc/internal/SqlParserFacade.java | 5 ++- .../src/main/javacc/ClickHouseSqlParser.jj | 4 ++ .../jdbc/PreparedStatementTest.java | 23 ++++++++++ .../internal/BaseSqlParserFacadeTest.java | 43 +++++++++++++++++++ 5 files changed, 80 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c92a57a..2e18890ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,13 @@ ### Bug Fixes +- **[jdbc-v2]** Fixed `Connection#prepareStatement` throwing a `NullPointerException` for an + `INSERT ... VALUES (...)` statement whose values list the default JavaCC parser cannot parse — most commonly one + containing a heredoc string (`$$...$$`), which the grammar has no token for, but also any other unparsable token + inside the list. The parser's error recovery left the values list's start position recorded without its matching end + position, which was then unboxed unguarded. Both positions are now dropped together, so the driver falls back to its + generic parameter-substitution path and such statements are prepared and executed successfully. The `ANTLR4` + parser backends were not affected. (https://github.com/ClickHouse/clickhouse-java/issues/3013) - **[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) 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 178c9a070..7c32df368 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,9 @@ 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 endIndexValue = parsedStmt.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_END); + if (startIndex != null && endIndexValue != null) { + int endIndex = endIndexValue; stmt.setAssignValuesListStartPosition(startIndex); stmt.setAssignValuesListStopPosition(endIndex); String query = parsedStmt.getSQL(); diff --git a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj index d0c088615..490221cf9 100644 --- a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj +++ b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj @@ -622,6 +622,10 @@ void dataClause(): {} { { token_source.format = token.image; } )? (anyExprList())? } catch (ParseException e) { // FIXME introduce a lexical state in next release with consideration of delimiter from the context + // The values list was abandoned mid-way, so its start/end positions can only be recorded partially. + // Drop both so consumers either get a complete pair or none at all. + token_source.removePosition(ClickHouseSqlStatement.KEYWORD_VALUES_START); + token_source.removePosition(ClickHouseSqlStatement.KEYWORD_VALUES_END); Token nextToken; do { nextToken = getNextToken(); diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java index 14c19f7a9..cb3f2408b 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java @@ -878,6 +878,29 @@ void testMetabaseBug01() throws Exception { } } + @Test(groups = { "integration" }) + void testInsertWithHeredocValue() throws Exception { + final String table = "test_insert_heredoc"; + try (Connection conn = getJdbcConnection()) { + try (Statement stmt = conn.createStatement()) { + stmt.execute("DROP TABLE IF EXISTS " + table); + stmt.execute("CREATE TABLE " + table + " (s String, n Int32) Engine MergeTree ORDER BY ()"); + } + try (PreparedStatement stmt = conn.prepareStatement( + "INSERT INTO " + table + " (s, n) VALUES ($$a@b$$, ?)")) { + stmt.setInt(1, 42); + assertEquals(stmt.executeUpdate(), 1); + } + try (Statement stmt = conn.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT s, n FROM " + table)) { + assertTrue(rs.next()); + assertEquals(rs.getString(1), "a@b"); + assertEquals(rs.getInt(2), 42); + assertFalse(rs.next()); + } + } + } + @Test(groups = { "integration" }) void testStatementSplit() throws Exception { try (Connection conn = getJdbcConnection()) { 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..f6294d25c 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 @@ -151,6 +151,49 @@ public static Object[][] testPreparedStatementInsertSQLDP() { }; } + @Test(dataProvider = "testInsertWithUnsupportedValuesListDP") + public void testInsertWithUnsupportedValuesList(String sql) { + ParsedPreparedStatement parsed = parser.parsePreparedStatement(sql); + assertTrue(parsed.isInsert(), "Should be of insert type"); + + int start = parsed.getAssignValuesListStartPosition(); + int stop = parsed.getAssignValuesListStopPosition(); + assertEquals(start > -1, stop > -1, "Values list start and stop positions should be both set or both unset"); + if (start > -1) { + assertTrue(stop > start, "Values list should stop after it starts"); + assertEquals(sql.charAt(start), '(', "Values list should start with an opening parenthesis"); + assertEquals(sql.charAt(stop), ')', "Values list should end with a closing parenthesis"); + } + } + + @DataProvider + public static Object[][] testInsertWithUnsupportedValuesListDP() { + return new Object[][] { + { "INSERT INTO t VALUES ($$?$$, ?)" }, + { "INSERT INTO t VALUES ($$a@b$$, ?)" }, + { "INSERT INTO t VALUES ($$a@b$$, ?);" }, + { "INSERT INTO t VALUES (?, )" }, + { "INSERT INTO t VALUES (@@, ?)" }, + { "INSERT INTO t VALUES (1, ?), (@@, ?)" }, + }; + } + + @Test(dataProvider = "testInsertValuesListPositionsDP") + public void testInsertValuesListPositions(String sql, int start, int stop) { + ParsedPreparedStatement parsed = parser.parsePreparedStatement(sql); + assertEquals(parsed.getAssignValuesListStartPosition(), start, "Values list start position does not match"); + assertEquals(parsed.getAssignValuesListStopPosition(), stop, "Values list stop position does not match"); + } + + @DataProvider + public static Object[][] testInsertValuesListPositionsDP() { + return new Object[][] { + { "INSERT INTO t VALUES (?, ?)", 21, 26 }, + { "INSERT INTO t (a, b) VALUES (1, ?)", 28, 33 }, + { "INSERT INTO t VALUES ($$x$$, ?)", 21, 30 }, + }; + } + @Test public void testStmtWithCasts() { String sql = "SELECT ?::integer, ?, '?:: integer' FROM table WHERE v = ?::integer"; // CAST(?, INTEGER) From 739d35f265a2b2429fce4c5ba147225e0120b4be Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:25:21 +0000 Subject: [PATCH 2/2] Drop the value group count of an abandoned values list too The JavaCC recovery of dataClause() already drops the partial values-list positions. It kept the group counter, which is incremented after the value group is read but before its closing parenthesis is matched, so a list abandoned between the two was still reported as one complete group. ConnectionImpl gates the beta RowBinary writer on a single group only, and that writer takes the bound parameters and drops the literals of the list. Reset the counter with the positions, as the ANTLR4 backends already do in discardValuesListOfRecoveredParseTree. --- jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj | 10 ++++++++-- .../jdbc/internal/BaseSqlParserFacadeTest.java | 5 +++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj index 490221cf9..ac86c4744 100644 --- a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj +++ b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj @@ -336,6 +336,10 @@ TOKEN_MGR_DECLS: { void incValueGroup() { this.valueGroups++; } + + void resetValueGroups() { + this.valueGroups = 0; + } } SKIP: { @@ -622,10 +626,12 @@ void dataClause(): {} { { token_source.format = token.image; } )? (anyExprList())? } catch (ParseException e) { // FIXME introduce a lexical state in next release with consideration of delimiter from the context - // The values list was abandoned mid-way, so its start/end positions can only be recorded partially. - // Drop both so consumers either get a complete pair or none at all. + // The values list was abandoned mid-way, so its start/end positions and its group count can only be + // recorded partially. Drop the positions so consumers either get a complete pair or none at all, and + // drop the group count so the list is not reported as a single complete group it may not be. token_source.removePosition(ClickHouseSqlStatement.KEYWORD_VALUES_START); token_source.removePosition(ClickHouseSqlStatement.KEYWORD_VALUES_END); + token_source.resetValueGroups(); Token nextToken; do { nextToken = getNextToken(); 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 52c540a8d..3eb3821f4 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 @@ -169,6 +169,10 @@ public void testInsertWithUnsupportedValuesList(String sql) { assertTrue(stop > start, "Values list should stop after it starts"); assertEquals(sql.charAt(start), '(', "Values list should start with an opening parenthesis"); assertEquals(sql.charAt(stop), ')', "Values list should end with a closing parenthesis"); + } else { + assertNotEquals(parsed.getAssignValuesGroups(), 1, "A values list of unknown extent should not be " + + "reported as a single complete group: consumers of a single group, like the RowBinary " + + "writer, take only the bound parameters and drop the literals of the list"); } } @@ -181,6 +185,7 @@ public static Object[][] testInsertWithUnsupportedValuesListDP() { { "INSERT INTO t VALUES (?, )" }, { "INSERT INTO t VALUES (@@, ?)" }, { "INSERT INTO t VALUES (1, ?), (@@, ?)" }, + { "INSERT INTO t VALUES (?, 'a' 'b')" }, }; }