Skip to content

Commit e2b23e0

Browse files
fix(client-v2): reject non-array/non-List QBit write values
A non-null QBit value that is neither a Java array nor a List fell through serializeQBitData to serializeArrayData, which writes no bytes for such a value, desynchronizing the RowBinary stream so the following columns are misread or rejected by the server. Reject it up-front with a clear IllegalArgumentException, consistent with the existing fixed-dimension validation and the Geometry reject-unsupported path. Addresses Cursor Bugbot review on #2939.
1 parent 0e1a6bb commit e2b23e0

3 files changed

Lines changed: 40 additions & 4 deletions

File tree

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,18 +480,25 @@ public static void serializeArrayData(OutputStream stream, Object value, ClickHo
480480
* count is validated up-front so a wrong-sized (including empty) vector fails fast on the client
481481
* with a clear message instead of a late server {@code SERIALIZATION_ERROR}, mirroring the
482482
* client-side length enforcement already applied to the other fixed-size type,
483-
* {@code FixedString(N)}.
483+
* {@code FixedString(N)}. A non-null value that is neither a Java array nor a {@code List} cannot
484+
* carry a vector and is rejected as well — otherwise it would fall through to
485+
* {@link #serializeArrayData} and write no bytes for the column, desynchronizing the
486+
* {@code RowBinary} stream and corrupting the columns that follow.
484487
*/
485488
private static void serializeQBitData(OutputStream stream, Object value, ClickHouseColumn column) throws IOException {
486489
if (value != null) {
487-
int length = -1;
490+
int length;
488491
if (value.getClass().isArray()) {
489492
length = Array.getLength(value);
490493
} else if (value instanceof List) {
491494
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());
492499
}
493500
int dimension = column.getPrecision();
494-
if (length >= 0 && length != dimension) {
501+
if (length != dimension) {
495502
throw new IllegalArgumentException("QBit column '" + column.getColumnName()
496503
+ "' expects exactly " + dimension + " elements but got " + length);
497504
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,35 @@ private Object[][] qbitWrongDimension() {
170170
};
171171
}
172172

173+
@Test(dataProvider = "qbitWrongType")
174+
public void testQBitSerializationRejectsNonArrayValue(Object value) {
175+
// A non-null QBit value that is neither a Java array nor a List cannot carry a vector. It must
176+
// be rejected up-front: otherwise it falls through to the Array serializer, which writes no
177+
// bytes for the column, desynchronizing the RowBinary stream and corrupting the following
178+
// columns. Writing into a byte sink so any (wrongly) emitted payload would be observable.
179+
ClickHouseColumn column = ClickHouseColumn.of("vec", "QBit(Float32, 8)");
180+
ByteArrayOutputStream out = new ByteArrayOutputStream();
181+
182+
IllegalArgumentException ex = Assert.expectThrows(IllegalArgumentException.class,
183+
() -> SerializerUtils.serializeData(out, value, column));
184+
Assert.assertTrue(ex.getMessage().contains("vec"),
185+
"Message should name the column: " + ex.getMessage());
186+
Assert.assertEquals(out.size(), 0,
187+
"Nothing should be written to the stream when the value is rejected");
188+
}
189+
190+
@DataProvider(name = "qbitWrongType")
191+
private Object[][] qbitWrongType() {
192+
// Values that are neither a Java array nor a List: a String, boxed scalars of the element
193+
// type, and a Map. None of these can represent a QBit(E, N) vector.
194+
return new Object[][] {
195+
{"not-a-vector"},
196+
{3.14f},
197+
{42d},
198+
{newMap("k", "v")},
199+
};
200+
}
201+
173202
@Test
174203
public void testQBitSerializationAcceptsExactDimensionAndMatchesArray() throws Exception {
175204
float[] vec = {1f, -2f, 3.5f, 4f, 5f, 6f, 7f, 8f};

docs/features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ Compatibility-sensitive traits:
4444
- Identifier quoting behavior is stable API for helper callers: identifiers are double-quoted, embedded double quotes are doubled, and optional quoting keeps simple identifiers unchanged.
4545
- Instant formatting is type-sensitive and should not drift: `Date` formatting depends on an explicit timezone, `DateTime` is serialized as epoch seconds, and higher-precision timestamps preserve up to 9 fractional digits.
4646
- `BFloat16` conversion is precision-sensitive and should not drift: a write keeps only the high 16 bits of the `float` (the low mantissa bits are truncated toward zero, matching the server's `Float32``BFloat16` conversion), so values that are not exactly representable in `BFloat16` change when written; a read widens the 16-bit value back to `float` losslessly.
47-
- `QBit` is wire-compatible with `Array(element_type)` and should not drift: the client transmits the logical vector as a length-prefixed array of its element type (`float[]` for `BFloat16`/`Float32`, `double[]` for `Float64`) rather than the server's bit-transposed on-disk layout, so a `QBit(E, N)` value read from or written to the server round-trips as an array of `E`. A written `QBit(E, N)` value must contain exactly `N` elements: a wrong-sized (including empty) vector is rejected during binary serialization with an `IllegalArgumentException` rather than deferred to a server error, matching the fixed dimension the server enforces.
47+
- `QBit` is wire-compatible with `Array(element_type)` and should not drift: the client transmits the logical vector as a length-prefixed array of its element type (`float[]` for `BFloat16`/`Float32`, `double[]` for `Float64`) rather than the server's bit-transposed on-disk layout, so a `QBit(E, N)` value read from or written to the server round-trips as an array of `E`. A written `QBit(E, N)` value must be a Java array or `List` holding exactly `N` elements: a wrong-sized (including empty) vector, or a non-null value that is neither an array nor a `List`, is rejected during binary serialization with an `IllegalArgumentException` rather than deferred to a server error or silently writing a misaligned stream, matching the fixed dimension the server enforces.
4848
- Timezone conversion helpers preserve nanoseconds and can intentionally shift local date or time when interpreted in a different timezone; this behavior is covered by tests and should not be normalized away.
4949
- `Geometry` handling is shape-sensitive: supported values are 1D through 4D Java arrays representing the nested geometry variants, and unsupported shapes or non-array values are rejected during serialization.
5050
- `Geometry` write inference is dimension-based rather than fully type-specific: point, ring/line string, polygon/multi-line string, and multi-polygon are selected from array depth, so writing `Geometry` cannot currently distinguish `Ring` from `LineString` or `Polygon` from `MultiLineString`.

0 commit comments

Comments
 (0)