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)