Description
ClickHouseUtils.skipSingleLineComment returns len (end of string) instead of the
index after the newline when the line comment is empty — i.e. when the newline sits
exactly at startIndex, as in --\n.
clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseUtils.java:1151
public static int skipSingleLineComment(String args, int startIndex, int len) {
int index = args.indexOf('\n', startIndex);
return index > startIndex ? index + 1 : len; // strict '>' is the defect
}
Its javadoc says it returns "index of start of next line, right after \n". Almost every
caller passes i + 2 (the character right after the 2-char -- marker), which for an
empty comment is exactly the newline position, so indexOf returns startIndex, the
index > startIndex test is false, and the scan jumps to the end of the query. Everything
after the empty comment is skipped.
Affected v1 callers that pass i + 2:
clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/JdbcParameterizedQuery.java:59 (? placeholder scan)
clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseParameterizedQuery.java:126 and :254 (named :param scan)
clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseRequest.java:131
- internal scanners in
ClickHouseUtils itself (lines 1105, 1222, 1263, 1319, 1428, 1475, 1507, 1582)
ClickHouseUtils.getLeadingComment (line 1028) passes the marker start index i, not
i + 2, so it is not affected.
Steps to reproduce
- Build
clickhouse-data, clickhouse-client and clickhouse-jdbc at main (40464dd).
- Parse a query that contains an empty
-- line comment between two placeholders.
- Observe that only the first placeholder is found and the rest of the query is dropped.
Error Log or Exception StackTrace
== helper directly ==
skipSingleLineComment("a--\nb", 3, 5) = 5 (expected 4)
skipSingleLineComment("a-- x\nb", 3, 7) = 6 (correct)
== clickhouse-jdbc v1 JdbcParameterizedQuery ==
"SELECT ? --\n, ?" params=1 applied=SELECT 1 --\n, ? <-- wrong
"SELECT ? -- x\n, ?" params=2 applied=SELECT 1 -- x\n, 2 <-- correct
"SELECT ?, ?" params=2 applied=SELECT 1, 2 <-- correct
== clickhouse-client ClickHouseParameterizedQuery (named) ==
"SELECT :a --\n, :b" params=[a] applied=SELECT 1 --\n, :b <-- wrong
"SELECT :a -- x\n, :b" params=[a, b] applied=SELECT 1 -- x\n, 2 <-- correct
The second placeholder is never registered, so the emitted SQL keeps a literal ? / :b,
and binding it through PreparedStatement fails with an out-of-range parameter index.
Expected Behaviour
An empty -- comment ends at its newline, exactly like a non-empty one. The server agrees
(ClickHouse 26.7.3.19):
$ printf 'SELECT 1 --\n, 2' | curl -s --data-binary @- http://localhost:8123/
1 2
So SELECT ? --\n, ? has two parameters, not one.
Code Example
ClickHouseConfig cfg = new ClickHouseConfig();
JdbcParameterizedQuery q = JdbcParameterizedQuery.of(cfg, "SELECT ? --\n, ?");
System.out.println(q.getParameters().size()); // prints 1, expected 2
StringBuilder sb = new StringBuilder();
q.apply(sb, new Object[] { 1, 2 });
System.out.println(sb); // prints "SELECT 1 --\n, ?", expected "SELECT 1 --\n, 2"
Suggested fix
One character in ClickHouseUtils.skipSingleLineComment, plus a data row in
ClickHouseUtilsTest.testSkipSingleLineComment:
int index = args.indexOf('\n', startIndex);
return index >= startIndex ? index + 1 : len; // index == -1 (no newline) still returns len
indexOf returns either -1 or a value >= startIndex, so >= keeps the
unterminated-comment case (-1) returning len unchanged, and only changes the
newline-at-startIndex case. Contrast case that must keep its current behavior: a comment
with no newline at all (SELECT ? --) still scans to the end of the query.
Unlike #3035 and #3037, this needs no parser redesign — it is a single comparison operator
in one shared helper, so it may be worth taking even though V1 is in maintenance.
Configuration
Environment
ClickHouse Server
- ClickHouse Server version: 26.7.3.19
- Non-default settings: none
- No tables required — reproduces with literal
SELECT.
Found by automated analysis of the client while working on the jdbc-v2 placeholder scan
(#3009 / PR #3010), and verified here against a live server, not by inspection alone.
Description
ClickHouseUtils.skipSingleLineCommentreturnslen(end of string) instead of theindex after the newline when the line comment is empty — i.e. when the newline sits
exactly at
startIndex, as in--\n.clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseUtils.java:1151Its javadoc says it returns "index of start of next line, right after
\n". Almost everycaller passes
i + 2(the character right after the 2-char--marker), which for anempty comment is exactly the newline position, so
indexOfreturnsstartIndex, theindex > startIndextest is false, and the scan jumps to the end of the query. Everythingafter the empty comment is skipped.
Affected v1 callers that pass
i + 2:clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/JdbcParameterizedQuery.java:59(?placeholder scan)clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseParameterizedQuery.java:126and:254(named:paramscan)clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseRequest.java:131ClickHouseUtilsitself (lines 1105, 1222, 1263, 1319, 1428, 1475, 1507, 1582)ClickHouseUtils.getLeadingComment(line 1028) passes the marker start indexi, noti + 2, so it is not affected.Steps to reproduce
clickhouse-data,clickhouse-clientandclickhouse-jdbcatmain(40464dd).--line comment between two placeholders.Error Log or Exception StackTrace
The second placeholder is never registered, so the emitted SQL keeps a literal
?/:b,and binding it through
PreparedStatementfails with an out-of-range parameter index.Expected Behaviour
An empty
--comment ends at its newline, exactly like a non-empty one. The server agrees(ClickHouse 26.7.3.19):
So
SELECT ? --\n, ?has two parameters, not one.Code Example
Suggested fix
One character in
ClickHouseUtils.skipSingleLineComment, plus a data row inClickHouseUtilsTest.testSkipSingleLineComment:indexOfreturns either-1or a value>= startIndex, so>=keeps theunterminated-comment case (
-1) returninglenunchanged, and only changes thenewline-at-startIndex case. Contrast case that must keep its current behavior: a comment
with no newline at all (
SELECT ? --) still scans to the end of the query.Unlike #3035 and #3037, this needs no parser redesign — it is a single comparison operator
in one shared helper, so it may be worth taking even though V1 is in maintenance.
Configuration
Environment
main, 40464dd)ClickHouse Server
SELECT.Found by automated analysis of the client while working on the jdbc-v2 placeholder scan
(#3009 / PR #3010), and verified here against a live server, not by inspection alone.