From d341a33832da97d33dcf86eded565bd476d8ea3a Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:57:19 +0000 Subject: [PATCH 1/2] Fix jdbc-v2: skip // comments and heredocs when scanning for ? placeholders SqlParserFacade.parseParameters knew '--'/'#' line comments, nested block comments and quoted strings, but not the '//' line comments and the $tag$...$tag$ heredocs that the server lexer also accepts. A '?' inside either was counted as a bind parameter, so PreparedStatement expected a value the application could not supply and executeQuery() failed with "Parameter at position 'N' is not set" for a query the server executes fine. The scan now treats '//' like the other line comment markers and skips a heredoc as an opaque token. A '$' is only a heredoc opener when it does not continue an identifier (a$b, a$x$), its tag contains word characters only, and a matching closing tag exists - otherwise it stays an ordinary character, matching the server lexer. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/3009 --- CHANGELOG.md | 6 +++ .../jdbc/internal/SqlParserFacade.java | 44 +++++++++++++++- .../jdbc/PreparedStatementTest.java | 25 +++++++++ .../internal/BaseSqlParserFacadeTest.java | 51 +++++++++++++++++++ 4 files changed, 125 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c92a57a..33db114bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,12 @@ ### Bug Fixes +- **[jdbc-v2]** Fixed a `?` inside a `//` line comment or inside a heredoc (dollar quoted string, e.g. `$$...$$` or + `$tag$...$tag$`) being counted as a `PreparedStatement` parameter. Such a statement expected a value the application + could not supply, so `executeQuery()` failed with `Parameter at position 'N' is not set` for a query the server + executes fine. The placeholder scan now skips both token kinds, like the server lexer does; a `$` that does not open a + heredoc is still treated as an ordinary character (it is a valid identifier character). + (https://github.com/ClickHouse/clickhouse-java/issues/3009) - **[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..c6fad0579 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 @@ -482,15 +482,57 @@ private static void parseParameters(String originalQuery, ParsedPreparedStatemen continue; } else if (i + 1 < len) { char nextCh = originalQuery.charAt(i + 1); - if ((ch == '-' && nextCh == ch) || (ch == '#')) { + if ((ch == '-' && nextCh == ch) || (ch == '/' && nextCh == ch) || (ch == '#')) { i = ClickHouseUtils.skipSingleLineComment(originalQuery, i + 2, len) - 1; } else if (ch == '/' && nextCh == '*') { i = ClickHouseUtils.skipMultiLineComment(originalQuery, i + 2, len) - 1; + } else if (ch == '$') { + i = skipHeredoc(originalQuery, i, len) - 1; } } } } + /** + * Skips a heredoc (dollar quoted string) like {@code $$...$$} or {@code $tag$...$tag$}, where the tag + * may only contain word characters. When there is no heredoc at {@code startIndex} the dollar sign is + * treated as an ordinary character, because it is also a valid identifier character: a dollar sign that + * follows a word character continues an identifier (e.g. {@code a$b} or {@code a$x$}) instead of opening + * a heredoc, and a dollar sign without a matching closing tag does not open one either. + * + * @param query non-null string to scan + * @param startIndex index of the dollar sign that may open a heredoc + * @param len end index, usually length of the given string + * @return index next to the closing tag, or {@code startIndex + 1} when there is no heredoc + */ + private static int skipHeredoc(String query, int startIndex, int len) { + if (startIndex > 0 && isWordChar(query.charAt(startIndex - 1))) { + return startIndex + 1; + } + + int tagEndIndex = query.indexOf('$', startIndex + 1); + if (tagEndIndex < 0 || tagEndIndex >= len) { + return startIndex + 1; + } + + for (int i = startIndex + 1; i < tagEndIndex; i++) { + if (!isWordChar(query.charAt(i))) { + return startIndex + 1; + } + } + + String tag = query.substring(startIndex, tagEndIndex + 1); + int closingTagIndex = query.indexOf(tag, tagEndIndex + 1); + if (closingTagIndex < 0 || closingTagIndex + tag.length() > len) { + return startIndex + 1; + } + return closingTagIndex + tag.length(); + } + + private static boolean isWordChar(char ch) { + return ch == '_' || (ch >= '0' && ch <= '9') || (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z'); + } + public enum SQLParser { /** 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..b33103310 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java @@ -935,6 +935,31 @@ void testStatementSplit() throws Exception { } } + @Test(groups = { "integration" }, dataProvider = "commentsAndHeredocsDP") + void testPlaceholdersWithCommentsAndHeredocs(String sql, String expected) throws Exception { + try (Connection conn = getJdbcConnection()) { + try (PreparedStatement stmt = conn.prepareStatement(sql)) { + stmt.setString(1, "42"); + try (ResultSet rs = stmt.executeQuery()) { + assertTrue(rs.next()); + assertEquals(rs.getString(1), expected); + assertFalse(rs.next()); + } + } + } + } + + @DataProvider(name = "commentsAndHeredocsDP") + public static Object[][] commentsAndHeredocsDP() { + return new Object[][] { + {"SELECT ? AS v // ?", "42"}, + {"SELECT ? AS v // ?\nUNION ALL SELECT NULL WHERE 0", "42"}, + {"SELECT concat($$?$$, ?) AS v", "?42"}, + {"SELECT concat($tag$ ? $tag$, ?) AS v", " ? 42"}, + {"SELECT ? AS a$x$, 1 AS b$x$", "42"}, + }; + } + @Test(groups = {"integration"}) void testClearParameters() throws Exception { final String sql = "insert into `test_issue_2299` (`id`, `name`, `age`) values (?, ?, ?)"; 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..2fb5d41f6 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 @@ -25,9 +25,12 @@ public abstract class BaseSqlParserFacadeTest { private final boolean javaCcBackend; + private final boolean paramsFromGrammarBackend; + public BaseSqlParserFacadeTest(String name) throws Exception { parser = SqlParserFacade.getParser(name, new JdbcConfiguration("jdbc:ch:http://localhost:8123", new Properties())); javaCcBackend = SqlParserFacade.SQLParser.JAVACC.name().equals(name); + paramsFromGrammarBackend = SqlParserFacade.SQLParser.ANTLR4_PARAMS_PARSER.name().equals(name); } @Test @@ -386,6 +389,54 @@ public static Object[][] testCTEStmtsDP() { }; } + @Test(dataProvider = "testCommentsAndHeredocsDP") + public void testCommentsAndHeredocs(String sql, int args) { + // The ANTLR4_PARAMS_PARSER backend collects placeholders from the grammar, whose lexer has no + // token for '//' comments and heredocs, so it is not covered by this scan. The other backends + // must agree with the server on which '?' is a placeholder. + if (paramsFromGrammarBackend) { + return; + } + ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); + Assert.assertEquals(stmt.getArgCount(), args, "Args mismatch for: " + sql); + } + + @DataProvider + public static Object[][] testCommentsAndHeredocsDP() { + return new Object[][] { + // '//' line comments + {"SELECT 1 // ?", 0}, + {"SELECT 1 //", 0}, + {"SELECT ? // ?\n, ?", 2}, + {"SELECT 1 // ? -- ? /* ? */ $$?$$\n, ?", 1}, + // heredocs (dollar quoted strings) + {"SELECT $$?$$ AS v", 0}, + {"SELECT $tag$ ? $tag$ AS v", 0}, + {"SELECT $1$ ? $1$ AS v", 0}, + {"SELECT $$$$ AS v, ?", 1}, + {"SELECT $$a$b$$ AS v, ?", 1}, + {"SELECT $t$ ?\n -- ?\n // ?\n /* ? */ $t$ AS v, ?", 1}, + {"SELECT $$?$$, ?, $$?$$", 1}, + {"SELECT $$it's ?$$ AS v, ?", 1}, + {"SELECT $$ /* ? $$ AS v, ?", 1}, + // '//' and heredoc markers that are not comments or heredocs + {"SELECT '// ?' AS v, ?", 1}, + {"SELECT '$$?$$' AS v, ?", 1}, + {"SELECT -- '// ?'\n?", 1}, + {"SELECT /* $$?$$ */ ?", 1}, + {"SELECT 4 / 2 AS v, ?", 1}, + {"SELECT ? AS a$b, ? AS c$d, 3", 2}, + {"SELECT ? AS a$x$, ? AS b$x$", 2}, + {"SELECT 1 AS a$x$, ?", 1}, + {"SELECT $$ ? AS v, ?", 2}, + // already supported comment styles keep working + {"SELECT 1 -- ?", 0}, + {"SELECT 1 # ?", 0}, + {"SELECT 1 #! ?", 0}, + {"SELECT /* ? /* ? */ ? */ ?", 1}, + }; + } + @Test(dataProvider = "testMiscStmtDp") public void testMiscStatements(String sql, int args) { ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); From a7707e3731904b6c68bcc96a3e8ebff911a5d67e Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 19 Aug 2026 08:08:41 +0000 Subject: [PATCH 2/2] Fix jdbc-v2: an empty line comment must not drop later ? placeholders ClickHouseUtils.skipSingleLineComment returns len when the newline sits exactly at its startIndex, which is the case for an empty line comment (`--\n`, `//\n`, `#\n`) because callers pass the index after the marker. The placeholder scan then jumped to the end of the query and lost every later placeholder, so `SELECT ? //\n, ?` reported one parameter instead of two. The server ends such a comment at its newline and executes the rest of the query. Skip line comments with a local helper that starts at the second marker character, so an empty comment ends at its own newline. Applies to all three markers the scan handles (`--`, `//`, `#`/`#!`). Reported by Cursor Bugbot on PR #3010. --- .../jdbc/internal/SqlParserFacade.java | 18 +++++++++++++++++- .../clickhouse/jdbc/PreparedStatementTest.java | 2 ++ .../jdbc/internal/BaseSqlParserFacadeTest.java | 17 +++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) 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 244a132d3..6ec0e1d91 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 @@ -501,7 +501,7 @@ private static void parseParameters(String originalQuery, ParsedPreparedStatemen } else if (i + 1 < len) { char nextCh = originalQuery.charAt(i + 1); if ((ch == '-' && nextCh == ch) || (ch == '/' && nextCh == ch) || (ch == '#')) { - i = ClickHouseUtils.skipSingleLineComment(originalQuery, i + 2, len) - 1; + i = skipLineComment(originalQuery, i + 1, len) - 1; } else if (ch == '/' && nextCh == '*') { i = ClickHouseUtils.skipMultiLineComment(originalQuery, i + 2, len) - 1; } else if (ch == '$') { @@ -511,6 +511,22 @@ private static void parseParameters(String originalQuery, ParsedPreparedStatemen } } + /** + * Skips a line comment ({@code --}, {@code //}, {@code #} or {@code #!}) up to and including the + * terminating newline. An empty comment is terminated by the newline that directly follows the comment + * marker, so scanning must continue on the next line instead of stopping at the end of the query. + * + * @param query non-null string to scan + * @param startIndex index of the second character of the comment marker, which is never a newline for + * {@code --} and {@code //}, and is the first comment character for {@code #} + * @param len end index, usually length of the given string + * @return index of the start of the next line, or {@code len} when the comment is not terminated + */ + private static int skipLineComment(String query, int startIndex, int len) { + int index = query.indexOf('\n', startIndex); + return index < 0 || index >= len ? len : index + 1; + } + /** * Skips a heredoc (dollar quoted string) like {@code $$...$$} or {@code $tag$...$tag$}, where the tag * may only contain word characters. When there is no heredoc at {@code startIndex} the dollar sign is 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 a45851a63..eb786c203 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java @@ -954,6 +954,8 @@ public static Object[][] commentsAndHeredocsDP() { return new Object[][] { {"SELECT ? AS v // ?", "42"}, {"SELECT ? AS v // ?\nUNION ALL SELECT NULL WHERE 0", "42"}, + {"SELECT //\n? AS v", "42"}, + {"SELECT --\n? AS v", "42"}, {"SELECT concat($$?$$, ?) AS v", "?42"}, {"SELECT concat($tag$ ? $tag$, ?) AS v", " ? 42"}, {"SELECT ? AS a$x$, 1 AS b$x$", "42"}, 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 ab7b8ae84..9508d45fb 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 @@ -409,6 +409,23 @@ public static Object[][] testCommentsAndHeredocsDP() { {"SELECT 1 //", 0}, {"SELECT ? // ?\n, ?", 2}, {"SELECT 1 // ? -- ? /* ? */ $$?$$\n, ?", 1}, + // an empty line comment ends at its own newline, so later placeholders are still counted + {"SELECT ? //\n, ?", 2}, + {"SELECT ? //\n// ?\n, ?", 2}, + {"SELECT ? //\n?", 2}, + {"SELECT ? --\n, ?", 2}, + {"SELECT ? -- ?\n--\n, ?", 2}, + {"SELECT ? #\n, ?", 2}, + {"SELECT ? #!\n, ?", 2}, + {"SELECT ? //\n--\n#\n, ?", 2}, + {"//\nSELECT ?", 1}, + // a comment that is never terminated still ends the scan + {"SELECT ? //\n", 1}, + {"SELECT ? --", 1}, + // a comment marker inside a string, a heredoc or a block comment does not start a comment + {"SELECT '--\n' AS v, ?", 1}, + {"SELECT $$//\n$$ AS v, ?", 1}, + {"SELECT ? /* --\n */, ?", 2}, // heredocs (dollar quoted strings) {"SELECT $$?$$ AS v", 0}, {"SELECT $tag$ ? $tag$ AS v", 0},