From 8d12b9da93a1f598b075b91c82cb06aaa2b1c639 Mon Sep 17 00:00:00 2001 From: piyush15102003 Date: Tue, 6 Oct 2026 18:54:39 +0530 Subject: [PATCH] Reject a non-array value instead of writing no bytes for the column Closes #3041. serializeArrayData guarded on "value is a List or an array" and had no else branch, so a non-null value of any other type returned having written nothing at all for the column - not even the var-int length 0. In RowBinary the next column's bytes are then read as this column's length prefix and the rest of the row is misframed: over RowBinary the server reports Code: 33 CANNOT_READ_ALL_DATA, and over RowBinaryWithDefaults the insert can succeed and store corrupted rows. Six dispatch paths reach it - Array, and the five array-backed geo types Ring/LineString/MultiPoint, Polygon/MultiLineString and MultiPolygon, whose unwrap is conditional, so a wrong-typed value stays un-unwrapped and lands in the same silent skip. One else branch covers all of them. The sibling container serializers already reject this: Tuple throws, Geometry throws, Map raises ClassCastException, and QBit was guarded for exactly this hazard in #2939. Array and the geo types were the only ones failing silently. The message follows serializerVariant/serializerGeometry and names the value class and the column type rather than the column name, because the geo paths pass synthetic columns (georing, geopolygin) whose names would be misleading. null still writes the var-int length 0, and arrays and Lists serialize exactly as before. --- .../internal/SerializerUtils.java | 6 ++ .../internal/SerializerUtilsTest.java | 56 +++++++++++++++++++ docs/features.md | 1 + history/latest/3041.md | 11 ++++ 4 files changed, 74 insertions(+) create mode 100644 history/latest/3041.md diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java index be24460a3..ac6acb890 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java @@ -550,6 +550,12 @@ public static void serializeArrayData(OutputStream stream, Object value, ClickHo } serializeData(stream, val, column.getNestedColumns().get(0)); } + } else { + // Without this the method would return having written nothing at all for the column - not even the + // var-int length - and the bytes of the next column would be read as this column's length prefix, + // misframing the rest of the row. + throw new IllegalArgumentException("Cannot write value of class " + value.getClass() + + " into column with array type " + column.getOriginalTypeName()); } } diff --git a/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java b/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java index 1680e8424..74f993a04 100644 --- a/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java @@ -792,6 +792,62 @@ public void testStringValueToByteArrayPassesThroughByteArray() { Assert.assertSame(SerializerUtils.stringValueToByteArray(bytes), bytes); } + @Test(dataProvider = "nonArrayValuesForArrayColumns", expectedExceptions = IllegalArgumentException.class) + public void testSerializeArrayDataRejectsNonArrayValue(String typeName, Object value) throws Exception { + SerializerUtils.serializeData(new ByteArrayOutputStream(), value, ClickHouseColumn.of("v", typeName)); + } + + @DataProvider(name = "nonArrayValuesForArrayColumns") + private Object[][] nonArrayValuesForArrayColumns() { + return new Object[][] { + {"Array(String)", "not-an-array"}, + {"Array(UInt32)", 42}, + {"Array(Array(String))", "not-an-array"}, + {"Ring", "not-an-array"}, + {"LineString", "not-an-array"}, + {"Polygon", "not-an-array"}, + {"MultiLineString", "not-an-array"}, + {"MultiPolygon", "not-an-array"}, + }; + } + + @Test + public void testSerializeArrayDataWritesNothingBeforeRejecting() throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + SerializerUtils.serializeData(out, 1, ClickHouseColumn.of("id", "UInt32")); + int afterId = out.size(); + + Assert.assertThrows(IllegalArgumentException.class, () -> + SerializerUtils.serializeData(out, "not-an-array", ClickHouseColumn.of("val", "Array(String)"))); + + Assert.assertEquals(out.size(), afterId); + } + + @Test + public void testSerializeArrayDataKeepsFollowingColumnAligned() throws Exception { + ClickHouseColumn id = ClickHouseColumn.of("id", "UInt32"); + ClickHouseColumn val = ClickHouseColumn.of("val", "Array(String)"); + ClickHouseColumn tail = ClickHouseColumn.of("tail", "String"); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + SerializerUtils.serializeData(out, 1, id); + SerializerUtils.serializeData(out, Arrays.asList("a", "b"), val); + SerializerUtils.serializeData(out, "TAIL", tail); + + BinaryStreamReader reader = newReader(out.toByteArray()); + reader.readValue(id); + reader.readValue(val); + Assert.assertEquals(reader.readValue(tail), "TAIL"); + } + + @Test + public void testSerializeArrayDataWritesEmptyLengthForNull() throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + SerializerUtils.serializeData(out, null, ClickHouseColumn.of("v", "Array(String)")); + + Assert.assertEquals(out.toByteArray(), new byte[] {0}); + } + private void assertCustomGeoTypeTag(String typeName) throws Exception { ByteArrayOutputStream out = new ByteArrayOutputStream(); SerializerUtils.writeDynamicTypeTag(out, ClickHouseColumn.of("v", typeName)); diff --git a/docs/features.md b/docs/features.md index b8b2b24a8..3d0d0d538 100644 --- a/docs/features.md +++ b/docs/features.md @@ -53,6 +53,7 @@ Compatibility-sensitive traits: - `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. - `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. Symmetrically, a `QBit(E, N)` read whose on-wire element count does not equal the declared dimension `N` is rejected with a `ClientException` rather than returning a wrong-length vector. - 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. +- Array-shaped columns reject a value they cannot represent: a non-`null` value written to an `Array` column, or to one of the array-backed geo types (`Ring`, `LineString`, `MultiPoint`, `Polygon`, `MultiLineString`, `MultiPolygon`), must be a Java array or a `List`. Anything else is rejected during binary serialization with an `IllegalArgumentException` naming the value class and the column type, rather than writing no bytes for the column - which previously left the following columns misframed in the `RowBinary` stream, surfacing as a server-side `CANNOT_READ_ALL_DATA` or, with `RowBinaryWithDefaults`, as a silently successful insert of corrupted rows. A `null` still writes the var-int length `0`, and arrays and `List`s serialize exactly as before. - `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. - `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`. `MultiPoint` shares the same 2D shape and is deliberately not selectable through the generic `Geometry` write path — a 2D value keeps resolving to `Ring`, so writing `MultiPoint` requires a concrete `MultiPoint` column. - Session precedence is part of the contract: client session defaults apply to each request, operation settings may override them, and only the client `session_id` is mutable at runtime while other client session properties remain fixed for the lifetime of the client. diff --git a/history/latest/3041.md b/history/latest/3041.md new file mode 100644 index 000000000..7c50a92b8 --- /dev/null +++ b/history/latest/3041.md @@ -0,0 +1,11 @@ +## Breaking changes + +- **[client-v2]** A non-`null` value written to an `Array` column, or to one of the array-backed + geo types (`Ring`, `LineString`, `MultiPoint`, `Polygon`, `MultiLineString`, `MultiPolygon`), + that is neither a Java array nor a `List` is now rejected with an `IllegalArgumentException`. + `serializeArrayData` previously wrote nothing at all for such a column - not even the var-int + length `0` - so the bytes of the next column were read as this column's length prefix and the + rest of the row was misframed. Over `RowBinary` that surfaced as a server-side `Code: 33 + CANNOT_READ_ALL_DATA`; over `RowBinaryWithDefaults` the insert could succeed and store corrupted + rows. `null` still writes the var-int length `0`, and arrays and `List`s are serialized exactly + as before. (https://github.com/ClickHouse/clickhouse-java/issues/3041)