From 2f486585a690bb59bf47441c15397d875f44edaf Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Tue, 4 Aug 2026 02:45:39 +0000 Subject: [PATCH 1/2] Fix jdbc-v2: discard values list positions that do not address the original SQL The JavaCC token manager records the INSERT VALUES list positions as offsets into the SQL it rebuilds from the token stream, where semicolons are dropped and JDBC escape sequences are rewritten or dropped, while PreparedStatementImpl slices the SQL the caller passed in. When the two have drifted apart the slice was taken at the wrong offsets, throwing StringIndexOutOfBoundsException out of prepareStatement or producing a truncated values list template that then threw out of addBatch. The positions are now verified against the original SQL and discarded when they do not delimit its values list, so the driver falls back to its generic parameter substitution path. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/3017 --- CHANGELOG.md | 10 ++++ .../jdbc/internal/SqlParserFacade.java | 60 +++++++++++++++++++ .../jdbc/PreparedStatementTest.java | 28 ++++++++- .../internal/BaseSqlParserFacadeTest.java | 43 +++++++++++++ 4 files changed, 139 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c92a57a..1e198510f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,16 @@ ### Bug Fixes +- **[jdbc-v2]** Fixed `Connection#prepareStatement` and `PreparedStatement#addBatch` throwing + `StringIndexOutOfBoundsException` for an `INSERT ... VALUES (...)` statement containing a JDBC escape sequence + (`{d '...'}`, `{ts '...'}`, ...) or a ClickHouse query parameter whose name starts with `d`/`t` (e.g. `{d:Int32}`). + The default `JAVACC` parser records the values list positions as offsets into the SQL it rebuilds from the token + stream, where such sequences are rewritten or dropped, while the driver slices the original SQL with them — so the + slice was taken at the wrong offsets or past the end of the statement. The positions are now checked against the + original SQL and discarded when they do not address its values list, in which case the driver falls back to its + generic parameter substitution path. Such a statement is now prepared without error; the escape sequence itself is + still sent to the server unchanged. The `ANTLR4` parser backends were not affected. + (https://github.com/ClickHouse/clickhouse-java/issues/3017) - **[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..0615d57e6 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 @@ -123,9 +123,69 @@ public ParsedPreparedStatement parsePreparedStatement(String sql) { stmt.setUseFunction(parsedStmt.isFuncUsed()); parseParameters(sql, stmt); + discardValuesListPositionsNotMatchingOriginalSql(sql, stmt); return stmt; } + /** + * The token manager records keyword positions as offsets into the SQL it rebuilds from the token stream, which + * is not always identical to the SQL it was given: semicolons are dropped and JDBC escape sequences are + * rewritten. Consumers of the values list positions slice the original SQL, so when the two have drifted apart + * the positions address the wrong characters or point past the end of the string. Discard them in that case to + * let the generic parameter substitution path handle the statement. + */ + private void discardValuesListPositionsNotMatchingOriginalSql(String sql, ParsedPreparedStatement stmt) { + int startPosition = stmt.getAssignValuesListStartPosition(); + int stopPosition = stmt.getAssignValuesListStopPosition(); + if (startPosition < 0 || stopPosition < 0) { + return; + } + + boolean matches = stopPosition > startPosition && stopPosition < sql.length() + && sql.charAt(startPosition) == '(' && closesParenthesizedGroup(sql, startPosition, stopPosition); + if (matches) { + int[] paramPositions = stmt.getParamPositions(); + for (int i = 0; i < stmt.getArgCount(); i++) { + if (paramPositions[i] < startPosition || paramPositions[i] > stopPosition) { + matches = false; + break; + } + } + } + + if (!matches) { + LOG.debug("Values list positions [{}, {}] do not match the original SQL", startPosition, stopPosition); + stmt.setAssignValuesListStartPosition(-1); + stmt.setAssignValuesListStopPosition(-1); + } + } + + /** + * Tells whether the parenthesis opened at {@code startPosition} is closed exactly at {@code stopPosition}, + * ignoring parentheses inside quoted text. + */ + private boolean closesParenthesizedGroup(String sql, int startPosition, int stopPosition) { + int depth = 0; + try { + for (int i = startPosition; i <= stopPosition; i++) { + char ch = sql.charAt(i); + if (ClickHouseUtils.isQuote(ch)) { + i = ClickHouseUtils.skipQuotedString(sql, i, sql.length(), ch) - 1; + if (i > stopPosition) { + return false; + } + } else if (ch == '(') { + depth++; + } else if (ch == ')' && --depth == 0) { + return i == stopPosition; + } + } + } catch (IllegalArgumentException e) { // unterminated quoted text + return false; + } + return false; + } + private List processRoles(Map settings) { String rolesCount = settings.get("_ROLES_COUNT"); if (rolesCount != null) { 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..6df70d46d 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,32 @@ void testMetabaseBug01() throws Exception { } } + @Test(groups = { "integration" }, dataProvider = "insertWithRewrittenValuesListDP") + void testInsertWithRewrittenValuesList(String valuesList) throws Exception { + final String table = "test_insert_rewritten_values_list"; + 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 " + valuesList)) { + assertEquals(stmt.getParameterMetaData().getParameterCount(), 1); + stmt.setInt(1, 42); + stmt.addBatch(); + } + } + } + + @DataProvider(name = "insertWithRewrittenValuesListDP") + public static Object[][] insertWithRewrittenValuesListDP() { + return new Object[][] { + { "(toDateTime({ts '2024-01-01 00:00:00'}), ?)" }, + { "(toTime({t '10:20:30'}), ?)" }, + { "(toInt32({d:Int32}), ?)" }, + }; + } + @Test(groups = { "integration" }) void testStatementSplit() throws Exception { try (Connection conn = getJdbcConnection()) { @@ -1106,7 +1132,6 @@ void testBatchInsertNoValuesReuse() throws Exception { stmt.setString(1, "invalid"); stmt.setInt(2, rnd.nextInt()); stmt.addBatch(); - assertThrows(SQLException.class, stmt::executeBatch); // should fail due to the previous batch data. assertThrows(SQLException.class, stmt::executeBatch); // clear previous batch data @@ -1160,7 +1185,6 @@ void testBatchInsertValuesReuse() throws Exception { // add a batch with invalid values stmt.setString(1, "invalid"); stmt.addBatch(); - assertThrows(SQLException.class, stmt::executeBatch); // should fail due to the previous batch data. assertThrows(SQLException.class, stmt::executeBatch); // clear previous batch data 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..df48f1b37 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 = "testValuesListPositionsDP") + public void testValuesListPositions(String sql, boolean positionsExpected) { + ParsedPreparedStatement parsed = parser.parsePreparedStatement(sql); + assertTrue(parsed.isInsert(), "Should be of insert type"); + + int start = parsed.getAssignValuesListStartPosition(); + int stop = parsed.getAssignValuesListStopPosition(); + if (parsed.getAssignValuesGroups() == 1 && start > -1 && stop > -1) { + assertTrue(stop > start, "Values list should stop after it starts, but got [" + start + ", " + stop + "]"); + assertTrue(stop < sql.length(), "Values list should stop within the statement, but got " + stop + + " for a statement of " + sql.length() + " characters"); + assertEquals(sql.charAt(start), '(', "Values list should start with an opening parenthesis"); + assertEquals(sql.charAt(stop), ')', "Values list should end with a closing parenthesis"); + + int[] paramPositions = parsed.getParamPositions(); + for (int i = 0; i < parsed.getArgCount(); i++) { + assertTrue(paramPositions[i] > start && paramPositions[i] < stop, "Parameter " + (i + 1) + + " at position " + paramPositions[i] + " should be inside the values list '" + + sql.substring(start, stop + 1) + "'"); + } + } + + if (javaCcBackend) { + assertEquals(start > -1 && stop > -1, positionsExpected, + "Values list positions should " + (positionsExpected ? "" : "not ") + "be reported"); + } + } + + @DataProvider + public static Object[][] testValuesListPositionsDP() { + return new Object[][] { + { "INSERT INTO t (a, b) VALUES (1, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1, ?);", true }, + { "INSERT INTO t (a, b) VALUES ('a)b', ?)", true }, + { "INSERT INTO t (a, b) VALUES (toDate({d '2024-01-01'}), ?)", true }, + { "INSERT INTO t (a, b) VALUES (toDateTime({ts '2024-01-01 00:00:00'}), ?)", false }, + { "INSERT INTO t (a, b) VALUES (toTime({t '10:20:30'}), ?)", false }, + { "INSERT INTO t (a, b) VALUES (toInt32({d:Int32}), ?)", false }, + { "INSERT INTO t (a, b) VALUES (?, toDate({d '2024-01-01'}))", false }, + { "INSERT INTO t (a, b) VALUES (?, toString({tt 'temp'}))", false }, + }; + } + @Test public void testStmtWithCasts() { String sql = "SELECT ?::integer, ?, '?:: integer' FROM table WHERE v = ?::integer"; // CAST(?, INTEGER) From c1eb9084f7a23859505fa951acdf39d31c383093 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Tue, 4 Aug 2026 03:39:03 +0000 Subject: [PATCH 2/2] Do not treat parentheses inside SQL comments as structural in the values list check closesParenthesizedGroup() scanned the original SQL for the parenthesis closing the values list without skipping comments, so a ( or ) inside a --, //, #, #! or /* */ comment was taken as structural. For a statement whose recorded positions DO address the original SQL correctly, that ended the group early and made the check discard them, sending the statement down the generic substitution path for no reason. Skip comments the same way the token manager that recorded the positions does, including nested /* */ blocks (which ClickHouse supports). --- .../jdbc/internal/SqlParserFacade.java | 25 ++++++++++++++----- .../internal/BaseSqlParserFacadeTest.java | 10 ++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) 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 0615d57e6..03e1641f5 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 @@ -162,25 +162,38 @@ private void discardValuesListPositionsNotMatchingOriginalSql(String sql, Parsed /** * Tells whether the parenthesis opened at {@code startPosition} is closed exactly at {@code stopPosition}, - * ignoring parentheses inside quoted text. + * ignoring parentheses inside quoted text and inside comments. The comment forms recognized here are the ones + * the token manager treats as comments as well: {@code --}, {@code //}, {@code #} (thus also {@code #!}) up to + * the end of the line, and nestable {@code /* ... *}{@code /} blocks. */ private boolean closesParenthesizedGroup(String sql, int startPosition, int stopPosition) { + int len = sql.length(); int depth = 0; try { for (int i = startPosition; i <= stopPosition; i++) { char ch = sql.charAt(i); if (ClickHouseUtils.isQuote(ch)) { - i = ClickHouseUtils.skipQuotedString(sql, i, sql.length(), ch) - 1; - if (i > stopPosition) { - return false; - } + i = ClickHouseUtils.skipQuotedString(sql, i, len, ch) - 1; + } else if (ch == '#' || (i + 1 < len && sql.charAt(i + 1) == ch && (ch == '-' || ch == '/'))) { + // search from the last character of the comment opener: it is never a line separator, and + // skipSingleLineComment() only reports one found strictly after the index it is given + i = ClickHouseUtils.skipSingleLineComment(sql, ch == '#' ? i : i + 1, len) - 1; + } else if (ch == '/' && i + 1 < len && sql.charAt(i + 1) == '*') { + i = ClickHouseUtils.skipMultiLineComment(sql, i + 2, len) - 1; } else if (ch == '(') { depth++; + continue; } else if (ch == ')' && --depth == 0) { return i == stopPosition; + } else { + continue; + } + + if (i > stopPosition) { // quoted text or comment reaching past the values list + return false; } } - } catch (IllegalArgumentException e) { // unterminated quoted text + } catch (IllegalArgumentException e) { // unterminated quoted text or comment return false; } return false; 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 df48f1b37..28bf9c059 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 @@ -185,7 +185,17 @@ public static Object[][] testValuesListPositionsDP() { { "INSERT INTO t (a, b) VALUES (1, ?)", true }, { "INSERT INTO t (a, b) VALUES (1, ?);", true }, { "INSERT INTO t (a, b) VALUES ('a)b', ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 /* ) */, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 -- )\n, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 // )\n, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 # )\n, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 #! )\n, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 --\n, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 /* ( */, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1 /* ? */, ?)", true }, + { "INSERT INTO t (a, b) VALUES (1, ? /* ) */)", true }, { "INSERT INTO t (a, b) VALUES (toDate({d '2024-01-01'}), ?)", true }, + { "INSERT INTO t (a, b) VALUES (toDateTime({ts '2024-01-01 00:00:00'}) /* ) */, ?)", false }, { "INSERT INTO t (a, b) VALUES (toDateTime({ts '2024-01-01 00:00:00'}), ?)", false }, { "INSERT INTO t (a, b) VALUES (toTime({t '10:20:30'}), ?)", false }, { "INSERT INTO t (a, b) VALUES (toInt32({d:Int32}), ?)", false },