Skip to content

Commit 557768b

Browse files
fix(client-v2): address QBit review — reject QBit in Native format, reject null QBit, await DDL in test
Review response on PR #2939: - NativeFormatReader: reject reading any column that is or contains a QBit (recursive check). The server transmits QBit in the Native format using its internal bit-transposed Tuple(FixedString(...)) layout, not the Array-like RowBinary representation this reader decodes, so the previous fall-through misread the bytes and desynchronized the block. Fail loud with a clear ClientException pointing to a RowBinary format instead of decoding garbage. - SerializerUtils.serializeQBitData: reject a null value. A fixed-dimension QBit cannot be null; a QBit nested in a container (Tuple/Map/Array) reaches this path via serializeNestedData without the top-level preamble, so without the guard the null was written as a zero-length vector, desynchronizing the stream. - DataTypeTests.writeReadVerify: await client.execute(tableDef) in the no-ddl branch (it was unawaited, racing the subsequent getTableSchema). - Tests: Native rejection (top-level + nested Map(String,QBit)); null QBit rejection (direct + nested-in-Tuple). docs/features.md + CHANGELOG updated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent e2b23e0 commit 557768b

6 files changed

Lines changed: 147 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
`BFloat16`/`Float32`, `double[]` for `Float64`) through generic records, binary readers, and POJO binding. In the
2020
JDBC driver (`jdbc-v2`) `QBit` maps to `java.sql.Types.ARRAY` and is returned as a `java.sql.Array` from
2121
`getObject`/`getArray`. Previously `QBit` was an unimplemented type constant and reading or writing such a column
22-
failed. (https://github.com/ClickHouse/clickhouse-java/issues/2610)
22+
failed. Reading `QBit` through the `Native` output format is not supported — the server transmits it there using a
23+
different internal layout — and fails fast with a clear error; use a `RowBinary` format instead.
24+
(https://github.com/ClickHouse/clickhouse-java/issues/2610)
2325
- **[client-v2, jdbc-v2]** Added TLS cipher suite selection. `Client.Builder.setSSLCipherSuites(String...)` (client-v2)
2426
and the comma-separated `ssl_cipher_suites` connection property (client-v2 and jdbc-v2) restrict the cipher suites
2527
enabled on secure connections; when unset, the transport defaults are used. Cipher-suite selection is independent of the

client-v2/src/main/java/com/clickhouse/client/api/data_formats/NativeFormatReader.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,21 @@ private boolean readBlock() throws IOException {
9191
names.add(column.getColumnName());
9292
types.add(column.getDataType().name());
9393

94+
if (containsQBit(column)) {
95+
// QBit is transmitted in the Native format using its internal bit-transposed
96+
// Tuple(FixedString(...)) layout, which is NOT the Array(element_type)-like
97+
// representation used in RowBinary (the only representation this reader decodes for
98+
// QBit). Reading it through the columnar/per-row paths below would misread those bytes
99+
// and desynchronize the block, corrupting the columns that follow. Fail loudly instead
100+
// of silently decoding garbage. This also covers a QBit nested inside another type
101+
// (e.g. Map(String, QBit(...))). QBit can be read through a RowBinary format.
102+
throw new ClientException("Reading column '" + column.getColumnName() + "' ("
103+
+ column.getOriginalTypeName() + ") from the Native format is not supported "
104+
+ "because it contains a QBit type: QBit is serialized in the Native format "
105+
+ "using an internal layout this reader does not decode. Use a RowBinary format "
106+
+ "(e.g. RowBinaryWithNamesAndTypes) to read QBit values");
107+
}
108+
94109
List<Object> values = new ArrayList<>(nRows);
95110
if (column.isArray()) {
96111
int[] sizes = new int[nRows];
@@ -116,6 +131,26 @@ private boolean readBlock() throws IOException {
116131
return true;
117132
}
118133

134+
/**
135+
* Returns {@code true} if {@code column} is a {@code QBit} or contains a {@code QBit} anywhere in
136+
* its nested type tree (e.g. {@code Array(QBit(...))}, {@code Tuple(..., QBit(...))},
137+
* {@code Map(String, QBit(...))}). {@code QBit} uses a different, internal wire layout in the
138+
* Native format than in RowBinary, so this reader cannot decode it and rejects such columns
139+
* up-front rather than misreading the block. {@code Nullable}/{@code LowCardinality} wrappers are
140+
* flags on the column, so a wrapped {@code QBit} still reports {@code dataType == QBit} here.
141+
*/
142+
private static boolean containsQBit(ClickHouseColumn column) {
143+
if (column.getDataType() == ClickHouseDataType.QBit) {
144+
return true;
145+
}
146+
for (ClickHouseColumn nested : column.getNestedColumns()) {
147+
if (nested != column && containsQBit(nested)) {
148+
return true;
149+
}
150+
}
151+
return false;
152+
}
153+
119154
private static class Block {
120155
final List<String> names;
121156
final List<String> types;

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -484,24 +484,37 @@ public static void serializeArrayData(OutputStream stream, Object value, ClickHo
484484
* carry a vector and is rejected as well — otherwise it would fall through to
485485
* {@link #serializeArrayData} and write no bytes for the column, desynchronizing the
486486
* {@code RowBinary} stream and corrupting the columns that follow.
487+
* <p>
488+
* A {@code null} value is rejected for the same reason: a fixed-dimension {@code QBit} cannot be
489+
* represented by {@code null}. A top-level {@code null} non-nullable {@code QBit} is already
490+
* rejected by
491+
* {@link com.clickhouse.client.api.data_formats.RowBinaryFormatSerializer#writeValuePreamble},
492+
* but a {@code QBit} nested inside a container ({@code Tuple}/{@code Map}/{@code Array}) is written
493+
* through {@link #serializeNestedData}, which does not route a non-nullable element through that
494+
* preamble; without this guard the {@code null} would delegate to {@link #serializeArrayData} and
495+
* be written as a zero-length vector (var-int {@code 0}), again desynchronizing the stream. (A
496+
* {@code Nullable(QBit)} {@code null} never reaches here: its null-marker is written earlier by the
497+
* preamble or by {@link #serializeNestedData}, which then return.)
487498
*/
488499
private static void serializeQBitData(OutputStream stream, Object value, ClickHouseColumn column) throws IOException {
489-
if (value != null) {
490-
int length;
491-
if (value.getClass().isArray()) {
492-
length = Array.getLength(value);
493-
} else if (value instanceof List) {
494-
length = ((List<?>) value).size();
495-
} else {
496-
throw new IllegalArgumentException("QBit column '" + column.getColumnName()
497-
+ "' expects a Java array or List of its element type but got "
498-
+ value.getClass().getName());
499-
}
500-
int dimension = column.getPrecision();
501-
if (length != dimension) {
502-
throw new IllegalArgumentException("QBit column '" + column.getColumnName()
503-
+ "' expects exactly " + dimension + " elements but got " + length);
504-
}
500+
if (value == null) {
501+
throw new IllegalArgumentException("QBit column '" + column.getColumnName()
502+
+ "' cannot be null; expected exactly " + column.getPrecision() + " elements");
503+
}
504+
int length;
505+
if (value.getClass().isArray()) {
506+
length = Array.getLength(value);
507+
} else if (value instanceof List) {
508+
length = ((List<?>) value).size();
509+
} else {
510+
throw new IllegalArgumentException("QBit column '" + column.getColumnName()
511+
+ "' expects a Java array or List of its element type but got "
512+
+ value.getClass().getName());
513+
}
514+
int dimension = column.getPrecision();
515+
if (length != dimension) {
516+
throw new IllegalArgumentException("QBit column '" + column.getColumnName()
517+
+ "' expects exactly " + dimension + " elements but got " + length);
505518
}
506519
serializeArrayData(stream, value, column);
507520
}

client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,47 @@ public void testQBitSerializationAcceptsExactDimensionAndMatchesArray() throws E
213213
Assert.assertEquals(qbitOut.toByteArray(), arrayOut.toByteArray());
214214
}
215215

216+
@Test
217+
public void testQBitSerializationRejectsNull() {
218+
// A QBit has a fixed dimension, so a null value cannot satisfy it. A top-level null
219+
// non-nullable QBit is already rejected by RowBinaryFormatSerializer.writeValuePreamble, but a
220+
// QBit nested inside a Tuple/Map/Array is serialized through serializeNestedData, which does
221+
// NOT route a non-nullable element through that preamble. Without an explicit guard the null
222+
// would delegate to the Array serializer and be written as a zero-length vector (var-int 0),
223+
// desynchronizing the RowBinary stream and corrupting the following columns. Writing into a
224+
// byte sink so any (wrongly) emitted payload would be observable.
225+
ClickHouseColumn column = ClickHouseColumn.of("vec", "QBit(Float32, 8)");
226+
ByteArrayOutputStream out = new ByteArrayOutputStream();
227+
228+
IllegalArgumentException ex = Assert.expectThrows(IllegalArgumentException.class,
229+
() -> SerializerUtils.serializeData(out, null, column));
230+
Assert.assertTrue(ex.getMessage().contains("vec"),
231+
"Message should name the column: " + ex.getMessage());
232+
Assert.assertTrue(ex.getMessage().contains("null"),
233+
"Message should state the value cannot be null: " + ex.getMessage());
234+
Assert.assertEquals(out.size(), 0,
235+
"Nothing should be written to the stream when a null QBit is rejected");
236+
}
237+
238+
@Test
239+
public void testQBitNestedInTupleRejectsNullElement() throws Exception {
240+
// Exercises the production-reachable path for the null guard: a non-nullable QBit nested in a
241+
// Tuple is written through serializeNestedData, which does NOT apply the top-level
242+
// writeValuePreamble null-into-non-nullable check to a non-nullable element. Without the guard
243+
// in serializeQBitData the null element would be written as a zero-length vector (var-int 0),
244+
// desynchronizing the stream and corrupting the rest of the row.
245+
ClickHouseColumn tuple = ClickHouseColumn.of("t", "Tuple(QBit(Float32, 8))");
246+
ByteArrayOutputStream out = new ByteArrayOutputStream();
247+
List<Object> tupleValue = Arrays.asList((Object) null);
248+
249+
IllegalArgumentException ex = Assert.expectThrows(IllegalArgumentException.class,
250+
() -> SerializerUtils.serializeData(out, tupleValue, tuple));
251+
Assert.assertTrue(ex.getMessage().contains("cannot be null"),
252+
"Message should explain the null QBit is rejected: " + ex.getMessage());
253+
Assert.assertEquals(out.size(), 0,
254+
"Nothing should be written when the nested null QBit element is rejected");
255+
}
256+
216257
@Test(dataProvider = "nonNullableEnumTypes")
217258
public void testNullIntoNonNullableEnumThrowsIllegalArgument(String typeName) {
218259
ClickHouseColumn column = ClickHouseColumn.of("bs_flag", typeName);

client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.clickhouse.client.api.query.QuerySettings;
1818
import com.clickhouse.client.api.sql.SQLUtils;
1919
import com.clickhouse.data.ClickHouseDataType;
20+
import com.clickhouse.data.ClickHouseFormat;
2021
import com.clickhouse.data.ClickHouseVersion;
2122
import lombok.AllArgsConstructor;
2223
import lombok.Data;
@@ -99,7 +100,7 @@ private <T> void writeReadVerify(String table, String tableDef, Class<T> dtoClas
99100
BiConsumer<List<T>, T> rowVerifier, CommandSettings ddlSettings) throws Exception {
100101
client.execute("DROP TABLE IF EXISTS " + table).get();
101102
if (ddlSettings == null) {
102-
client.execute(tableDef);
103+
client.execute(tableDef).get();
103104
} else {
104105
client.execute(tableDef, ddlSettings).get();
105106
}
@@ -198,6 +199,42 @@ public void testQBit() throws Exception {
198199
}, ddl);
199200
}
200201

202+
@Test(groups = {"integration"})
203+
public void testQBitNativeFormatRejected() throws Exception {
204+
if (isVersionMatch(QBIT_UNSUPPORTED_VERSIONS)) {
205+
throw new SkipException("QBit requires ClickHouse 25.10+");
206+
}
207+
208+
// In the Native format the server transmits a QBit column using its internal bit-transposed
209+
// layout, which is NOT the Array(element_type)-like representation the client decodes for QBit
210+
// over RowBinary. Reading QBit via Native must therefore fail loudly with a clear error rather
211+
// than silently decoding garbage and misaligning the trailing column that follows it.
212+
QuerySettings settings = new QuerySettings()
213+
.setFormat(ClickHouseFormat.Native)
214+
.serverSetting("allow_experimental_qbit_type", "1");
215+
try (QueryResponse response = client.query(
216+
"SELECT CAST([1, 2, 3, 4, 5, 6, 7, 8] AS QBit(Float32, 8)) AS q, 42 AS tail", settings).get()) {
217+
ClientException ex = Assert.expectThrows(ClientException.class,
218+
() -> client.newBinaryFormatReader(response));
219+
Assert.assertTrue(ex.getMessage().contains("QBit"),
220+
"Expected a clear QBit message, got: " + ex.getMessage());
221+
Assert.assertTrue(ex.getMessage().contains("Native"),
222+
"Expected the message to mention the Native format, got: " + ex.getMessage());
223+
}
224+
225+
// The same rejection applies to a QBit nested inside another type (here Map(String, QBit)),
226+
// which the server does support and would otherwise be misread column-by-column.
227+
try (QueryResponse response = client.query(
228+
"SELECT CAST(map('a', [1, 2, 3]) AS Map(String, QBit(Float32, 3))) AS m", settings).get()) {
229+
ClientException ex = Assert.expectThrows(ClientException.class,
230+
() -> client.newBinaryFormatReader(response));
231+
Assert.assertTrue(ex.getMessage().contains("QBit"),
232+
"Expected a clear QBit message for the nested case, got: " + ex.getMessage());
233+
Assert.assertTrue(ex.getMessage().contains("Native"),
234+
"Expected the message to mention the Native format, got: " + ex.getMessage());
235+
}
236+
}
237+
201238
@Test(groups = {"integration"})
202239
public void testNestedDataTypes() throws Exception {
203240
final String table = "test_nested_types";

docs/features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ This document lists stable, user-visible behavior in `client-v2` and `jdbc-v2` t
2121
- JSONEachRow text reader: Can stream `JSONEachRow` responses through a caller-supplied `JsonParser`, with Jackson and Gson parser factory implementations available as optional classpath dependencies, and infers a best-effort schema from the first row.
2222
- Data type conversion: Maps ClickHouse types to Java values for binary reads, POJO binding, and SQL parameter formatting, including date/time handling.
2323
- BFloat16 type support: For ClickHouse `24.11+`, reads and writes the `BFloat16` type through generic records, binary readers, POJO binding, and `Nullable`/`Dynamic`/`Variant` wrappers. `BFloat16` maps to the Java `float` type: a read widens the stored 16-bit value losslessly, while a write keeps the high 16 bits of the `float`. In `jdbc-v2`, `BFloat16` maps to `java.sql.Types.FLOAT` / `java.lang.Float` and is read and written through the standard `getFloat`/`setFloat` and `getObject` accessors.
24-
- QBit type support: For ClickHouse `25.10+` (requires the `allow_experimental_qbit_type` server setting to create a column), reads and writes the experimental `QBit(element_type, dimension)` vector type, where `element_type` is `BFloat16`, `Float32`, or `Float64`. On the wire a `QBit` value is encoded exactly like `Array(element_type)` (a length-prefixed list of elements), so the client reads and writes it as a Java array of the element type — `float[]` for `BFloat16`/`Float32`, `double[]` for `Float64` — through generic records, binary readers, and POJO binding, using the same code path as `Array(element_type)`. The bit-transposed on-disk layout is a server storage detail and is not exposed to the client.
24+
- QBit type support: For ClickHouse `25.10+` (requires the `allow_experimental_qbit_type` server setting to create a column), reads and writes the experimental `QBit(element_type, dimension)` vector type, where `element_type` is `BFloat16`, `Float32`, or `Float64`. On the wire a `QBit` value is encoded exactly like `Array(element_type)` (a length-prefixed list of elements), so the client reads and writes it as a Java array of the element type — `float[]` for `BFloat16`/`Float32`, `double[]` for `Float64` — through generic records, binary readers, and POJO binding, using the same code path as `Array(element_type)`. This `Array`-like encoding is what the server uses over `RowBinary` formats; the `Native` format instead transmits `QBit` using its internal bit-transposed `Tuple(FixedString(...))` layout, which the client does not decode, so reading any column that is or contains a `QBit` (including a nested `QBit`, e.g. `Map(String, QBit(...))`) through the `Native` format is rejected with a clear `ClientException` (use a `RowBinary` format such as `RowBinaryWithNamesAndTypes` instead).
2525
- Geometry type support: For ClickHouse `25.11+`, where `Geometry` changed from a string alias to `Variant(Point, Ring, LineString, MultiLineString, Polygon, MultiPolygon)`, the client reads and writes `Geometry` values through generic records, binary readers, POJO binding, and SQL parameter formatting, using Java array dimensionality to represent the geometry shape.
2626
- Insert APIs: Supports inserting registered POJOs, raw streams, and callback-driven writers, with optional column lists and format selection.
2727
- Insert controls: Supports insert-specific settings such as deduplication token, query id, compression behavior, and request headers.

0 commit comments

Comments
 (0)