clickhouse-data (v1): ClickHouseUtils.skipSingleLineComment jumps to end of query for an empty -- comment, dropping the rest of the SQL

未关闭 适合新手
#3,066 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

评估

难度
1/5
预计耗时
1-3 小时
新手友好度
92/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
java, sql
领域
database

调研方向

从 clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseUtils.java 中的 skipSingleLineComment 开始,并检查 ClickHouseUtilsTest.testSkipSingleLineComment。添加 issue 中描述的空注释情况,验证没有换行符的注释仍会返回 len,然后运行聚焦的 utility 测试和受影响的 client/JDBC 测试;完成的标准是能够发现并绑定空 -- 注释之后的占位符。

由索引模型根据 Issue 内容生成。

描述

area:sql-parser bug client-v1

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
  1. Build clickhouse-data, clickhouse-client and clickhouse-jdbc at main (40464dd).
  2. Parse a query that contains an empty -- line comment between two placeholders.
  3. 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 小时
30 天内合并 PR
28

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

ClickHouse/clickhouse-java 的其他 Issue

查看 ClickHouse/clickhouse-java 的全部 Issue

相似的 Issue

更多 Java Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。