Fix jdbc-v2: discard values list positions that do not address the original SQL

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)

jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java

@@ -123,9 +123,82 @@
 
             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 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, 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 or comment
+                return false;
+            }
+            return false;
+        }
+
         private List<String> processRoles(Map<String, String> settings) {
             String rolesCount = settings.get("_ROLES_COUNT");
             if (rolesCount != null) {

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

jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java

@@ -151,6 +151,59 @@ 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 (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 },
+                { "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)