[jdbc-v2] RowBinary writer drops values set with setBytes, the stream setters and setURL (column gets NULL or its default)
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 初心者へのやさしさ
- 67/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- java
調査の方向性
Start with the empty setter overrides in jdbc-v2/src/main/java/com/clickhouse/jdbc/WriterStatementImpl.java named in the report, then inspect the existing character stream encoding path in PreparedStatementImpl and the writer’s value methods. Add regression coverage for the listed setters with RowBinary enabled, including executeUpdate() and batch inserts, and verify that bound values are stored while setNull still stores NULL or the column default.
索引モデルが issue の本文から書いたものです。
説明
Description
When the beta RowBinary writer is on (beta.row_binary_for_simple_insert=true), these PreparedStatement setters do not give the value to the writer:
setBytes(int, byte[])setAsciiStream(3 overloads),setUnicodeStreamsetBinaryStream(3 overloads)setCharacterStream(3 overloads),setNCharacterStream(2 overloads)setURL(int, URL)
The insert then writes no value for that column:
Nullablecolumn:NULLis stored. There is no error.- Column with a
DEFAULT: the default is stored. There is no error. - Other columns:
IllegalArgumentException: An attempt to write null into not nullable column 'v'. The message does not tell the user that the bound value was lost.
The first two cases are silent data loss. When the writer is off (PreparedStatementImpl), the same calls store the bound value. Both executeUpdate() and addBatch() + executeBatch() are affected.
The writer is opt-in now (default false). #3328 proposes to make it the default. Then every user of these setters would get this behavior. This is item 1 (and item 2, see "Related gap" below) from https://github.com/ClickHouse/clickhouse-java/issues/3328#issuecomment-6788937926.
Found by automated analysis of the client while working on #3328. Verified against a live server (not by inspection only).
Steps to reproduce
- Open a jdbc-v2 connection with
beta.row_binary_for_simple_insert=true. CREATE TABLE t_writer_lost (id Int32, b Nullable(String), d String DEFAULT 'x') ENGINE = MergeTree ORDER BY id- Prepare
INSERT INTO t_writer_lost VALUES (?, ?, ?). Bind columnbwithsetBytesand columndwithsetBinaryStreamorsetCharacterStream. RunexecuteUpdate()andexecuteBatch(). SELECT id, b, d FROM t_writer_lost ORDER BY id
Error Log or Exception StackTrace
Output of the code example below (main f9586aba0, ClickHouse 26.9.1.1629):
writer=false impl=PreparedStatementImpl
(1, abc, yz)
(2, def, uv)
writer=true impl=WriterStatementImpl
(1, null, x)
(2, null, x)
Each of the 14 setters was also checked alone, in a new table per row (INSERT INTO t VALUES (?, ?), value "abc", or http://example.com/a for setURL). The value was read back with SELECT v IS NULL, toString(v):
| Target column | Writer off | Writer on |
|---|---|---|
v Nullable(String) |
'abc' (13 setters, setURL: see note) |
NULL (all 14 setters) |
v String DEFAULT 'x' |
'abc' (13 setters, setURL: see note) |
'x' (all 14 setters) |
v String |
'abc' (13 setters, setURL: see note) |
IllegalArgumentException: An attempt to write null into not nullable column 'v' (all 14 setters) |
The control setString stores 'abc' in all six cells. Note: when the writer is off, setURL fails with a server syntax error (Code: 62). That is a different defect in the SQL-text path, and this issue does not include it.
Expected Behaviour
The column gets the bound value, the same as when the writer is off. For example, setBytes(2, "abc".getBytes()) into a Nullable(String) column stores abc, not NULL. setBinaryStream(3, …"yz"…) into String DEFAULT 'x' stores yz, not x.
Root cause
jdbc-v2/src/main/java/com/clickhouse/jdbc/WriterStatementImpl.java (main f9586aba0). These overrides only call ensureOpen(). Their body is empty otherwise:
setBytes(:205)setAsciiStream(:226, :231, :237),setUnicodeStream(:243)setBinaryStream(:248, :254, :260)setCharacterStream(:266, :271, :276),setNCharacterStream(:281, :287)setURL(:406)
Thus the row slot in the writer stays null. RowBinaryFormatWriter.commitRow() (client-v2/.../RowBinaryFormatWriter.java:89) gives null to RowBinaryFormatSerializer.writeValuePreamble() (RowBinaryFormatSerializer.java:199). That method writes the NULL marker for a Nullable column and the "use default" marker for a column with a default (RowBinaryWithDefaults). For other columns it throws.
Suggested fix
Give the value to the writer in each setter:
setBytes:writer.setValue(parameterIndex, x).SerializerUtils.serializeDataalready writes abyte[]forString/FixedString(SerializerUtils.java:711-726).setURL:writer.setString(parameterIndex, x == null ? null : x.toString()).InputStreamsetters: read the stream (up tolengthwhen it is given) into abyte[], thenwriter.setValue(...).Readersetters: read into aString, thenwriter.setString(...).PreparedStatementImpl.encodeCharacterStream()(:972) shows the read loop of the SQL-text path.
Do not use writer.setInputStream(...) / writer.setReader(...) (as setBlob / setClob do) for this fix. In the same test, setBlob(int, InputStream, long) with the writer on stored the text com.clickhouse.client.api.data_formats.RowBinaryFormatWriter$InputStreamHolder@3d1cfad4 in the column. That is a separate defect in client-v2 RowBinaryFormatWriter.setInputStream() / setReader().
Behavior that must not change: setNull must still store NULL or the column default. A regression test can bind each setter into Nullable(String), String DEFAULT 'x' and String with the writer on, and compare the result with the writer off.
Related gap (fails with an error, no data loss)
When the writer is on, setObject(int, Object, int), setObject(int, Object, int, int), setObject(int, Object, SQLType) and setObject(int, Object, SQLType, int) throw SQLException: This form of setObject is not supported yet (SQL state 0A000), WriterStatementImpl.java:424-451. When the writer is off, the same calls store the value. Verified in the same run with setObject(2, "abc", Types.VARCHAR), setObject(2, "abc", Types.VARCHAR, 0), setObject(2, "abc", JDBCType.VARCHAR) and setObject(2, "abc", JDBCType.VARCHAR, 0). Frameworks use these overloads (for example Spring JdbcTemplate with SqlParameterValue). This becomes important if #3328 makes the writer the default.
Code Example
import java.io.ByteArrayInputStream;
import java.io.StringReader;
import java.sql.*;
import java.util.Properties;
public class Minimal {
public static void main(String[] args) throws Exception {
com.clickhouse.jdbc.Driver.load();
for (boolean writer : new boolean[] {false, true}) {
Properties p = new Properties();
p.setProperty("user", "default");
p.setProperty("password", "<password>");
p.setProperty("beta.row_binary_for_simple_insert", String.valueOf(writer));
try (Connection conn = DriverManager.getConnection("jdbc:clickhouse://localhost:8123/default", p)) {
try (Statement st = conn.createStatement()) {
st.execute("DROP TABLE IF EXISTS t_writer_lost");
st.execute("CREATE TABLE t_writer_lost (id Int32, b Nullable(String), d String DEFAULT 'x') ENGINE = MergeTree ORDER BY id");
}
try (PreparedStatement ps = conn.prepareStatement("INSERT INTO t_writer_lost VALUES (?, ?, ?)")) {
ps.setInt(1, 1);
ps.setBytes(2, "abc".getBytes());
ps.setBinaryStream(3, new ByteArrayInputStream("yz".getBytes()));
ps.executeUpdate();
ps.setInt(1, 2);
ps.setBytes(2, "def".getBytes());
ps.setCharacterStream(3, new StringReader("uv"));
ps.addBatch();
ps.executeBatch();
System.out.println("writer=" + writer + " impl=" + ps.getClass().getSimpleName());
}
try (Statement st = conn.createStatement();
ResultSet rs = st.executeQuery("SELECT id, b, d FROM t_writer_lost ORDER BY id")) {
while (rs.next()) {
System.out.println(" (" + rs.getInt(1) + ", " + rs.getString(2) + ", " + rs.getString(3) + ")");
}
}
}
}
}
}
Configuration
Client Configuration
Properties p = new Properties();
p.setProperty("beta.row_binary_for_simple_insert", "true");
Environment
- Cloud
- Client version: jdbc-v2
0.12.0-rc1-SNAPSHOT, built from mainf9586aba0 - Language version: Java 17 (Temurin 17.0.21)
- OS: Linux (Docker)
ClickHouse Server
- ClickHouse Server version: 26.9.1.1629
- ClickHouse Server non-default settings, if any: none
CREATE TABLEstatements for tables involved:CREATE TABLE t_writer_lost (id Int32, b Nullable(String), d String DEFAULT 'x') ENGINE = MergeTree ORDER BY id- Sample data for all these tables: not needed, the example inserts the rows
- 主要言語
- Java
- スター
- 1.6k
- フォーク
- 649
- 平均マージ
- 1日 20時間
- マージ済み PR(30日)
- 39
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートあり
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
ClickHouse/clickhouse-java のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
ClickHouse/clickhouse-java#3349 ·
メンテナーはふだん 1 日以内に返信
-
bug jdbc-v2
難易度 2/5 1〜3時間 初心者へのやさしさ 85/100
ClickHouse/clickhouse-java#3347 ·
メンテナーはふだん 1 日以内に返信
-
bug jdbc-v2
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
ClickHouse/clickhouse-java#3326 ·
メンテナーはふだん 1 日以内に返信
-
[client-v2] Enum8/Enum16 insert accepts a non-member integer and uses Enum.ordinal(), silently storing values that are not enum members対応中かも @jaya232 が 6 日前に担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
ClickHouse/clickhouse-java#3304 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
bug jdbc-v2
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
ClickHouse/clickhouse-java#3214 ·
メンテナーはふだん 1 日以内に返信
ClickHouse/clickhouse-java の issue をすべて見る
似ている issue
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
メンテナーはふだん 1 日以内に返信
-
bug:unconfirmed
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
UniversalMediaServer/UniversalMediaServer#6473 ·
メンテナーはふだん 1 日以内に返信
-
難易度 1/5 1時間未満 初心者へのやさしさ 82/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
StarRocks/starrocks#83246 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
メンテナーはふだん 2 日以内に返信