clickhouse-data (v1): ClickHouseUtils.skipSingleLineComment jumps to end of query for an empty -- comment, dropping the rest of the SQL
まだ誰も着手していません。
評価
- 難易度
- 1/5
- 見積もり時間
- 1〜3時間
- 初心者へのやさしさ
- 92/100
調査の方向性
clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseUtils.java の skipSingleLineComment から始め、ClickHouseUtilsTest.testSkipSingleLineComment を確認します。issue に記載された空のコメントのケースを追加し、改行のないコメントでも引き続き len が返されることを検証してから、対象を絞った utility テストと影響を受ける client/JDBC テストを実行します。完了とは、空の -- コメントの後にあるプレースホルダーが検出され、バインドされることです。
索引モデルが issue の本文から書いたものです。
説明
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:126and:254(named:paramscan)clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseRequest.java:131- internal scanners in
ClickHouseUtilsitself (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-clientandclickhouse-jdbcatmain(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
- Cloud
- Client version: 0.10.0-rc1-SNAPSHOT (
main, 40464dd) - Language version: JDK 17
- OS: Linux (Docker)
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.
- 主要言語
- Java
- スター
- 1.6k
- フォーク
- 637
- 平均マージ
- 2日 12時間
- マージ済み PR(30日)
- 28
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
ClickHouse/clickhouse-java のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
ClickHouse/clickhouse-java#3143 ·
-
難易度 1/5 1時間未満 初心者へのやさしさ 85/100
ClickHouse/clickhouse-java#3111 ·
-
area:data-type bug
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
ClickHouse/clickhouse-java#3098 · コメント 1 件 ·
-
bug client-api-v2 test
難易度 2/5 1〜3時間 初心者へのやさしさ 92/100
ClickHouse/clickhouse-java#3076 ·
-
area:general bug client-api-v2 jdbc jdbc-v2
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
ClickHouse/clickhouse-java#3063 ·
ClickHouse/clickhouse-java の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
infinispan/infinispan#18150 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
-
untriaged
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
opensearch-project/k-NN#3597 ·
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100