From 9209449f4af7d89f6dbd207da8bf5a153740c3ac Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Fri, 17 Jul 2026 23:20:46 +0000 Subject: [PATCH 1/3] Fix client-v2: serialize SimpleAggregateFunction columns in RowBinary writer SerializerUtils.serializeData had no case for SimpleAggregateFunction, so inserting into such a column threw UnsupportedOperationException ("Unsupported data type: SimpleAggregateFunction") even though BinaryStreamReader already reads these columns. Delegate to the underlying type via serializeNestedData so a Nullable underlying still gets its RowBinary null-marker byte, mirroring the read path. Related to: https://github.com/ClickHouse/clickhouse-java/issues/2477 --- CHANGELOG.md | 6 +++ .../internal/SerializerUtils.java | 6 +++ .../internal/SerializerUtilsTest.java | 41 +++++++++++++++++++ .../datatypes/RowBinaryFormatWriterTest.java | 18 ++++---- 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebd7bba1f..ca7c339fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ ### Bug Fixes +- **[client-v2]** Fixed the `RowBinary` writer throwing `UnsupportedOperationException: Unsupported data type: + SimpleAggregateFunction` when inserting into a `SimpleAggregateFunction(func, T)` column (the reader already + supported these columns). The value is now serialized identically to its underlying type `T`, writing the + `Nullable` null-marker byte when the underlying type is nullable (e.g. `SimpleAggregateFunction(anyLast, + Nullable(String))`), mirroring the read path. (https://github.com/ClickHouse/clickhouse-java/issues/2477) + - **[client-v2]** Fixed binary array decoding for nullable element types so `Array(Nullable(Float64))` and similar columns now return boxed arrays such as `Double[]` instead of `Object[]`. This keeps null-supporting arrays aligned with their element type while preserving the existing `Object[]` fallback for Variant/Dynamic/Geometry arrays. (https://github.com/ClickHouse/clickhouse-java/issues/2846) - **[client-v2]** Fixed `Float32`/`Float64` columns throwing `ClassCastException` when a value of a 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 6ff4b4b79..73d08198a 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 @@ -72,6 +72,12 @@ public static void serializeData(OutputStream stream, Object value, ClickHouseCo case Map: serializeMapData(stream, value, column); break; + case SimpleAggregateFunction: + // A SimpleAggregateFunction(func, T) value serializes identically to its underlying + // type T; delegate through serializeNestedData so a Nullable underlying still gets + // its RowBinary null-marker byte, mirroring BinaryStreamReader's read path. + serializeNestedData(stream, value, column.getNestedColumns().get(0)); + break; case AggregateFunction: serializeAggregateFunction(stream, value, column); break; 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 265cd1cfb..f42d6ede4 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 @@ -217,6 +217,47 @@ private Object[][] nestedNullableData() throws Exception { }; } + @Test(dataProvider = "simpleAggregateFunctionData") + public void testSimpleAggregateFunctionRoundTrip(String typeName, Object value) throws Exception { + ClickHouseColumn column = ClickHouseColumn.of("v", typeName); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + SerializerUtils.serializeData(out, value, column); + + Object actual = newReader(out.toByteArray()).readValue(column); + Assert.assertEquals(normalize(actual), normalize(value)); + } + + @DataProvider(name = "simpleAggregateFunctionData") + private Object[][] simpleAggregateFunctionData() { + return new Object[][] { + // Top-level SAF columns - the exact shape reported in the bug, reached directly + // through the serializeData switch's SimpleAggregateFunction case. + {"SimpleAggregateFunction(sum, UInt64)", BigInteger.valueOf(42)}, + {"SimpleAggregateFunction(anyLast, Nullable(String))", "present"}, + + // A SimpleAggregateFunction(func, T) value serializes byte-identically to its + // underlying type T. Each SAF below sits in the MIDDLE of the schema between a + // leading Int32 and a trailing Float64, so a dropped or extra byte (such as a + // wrongly written null-marker) shifts the trailing Float64 and is detected + // positionally. The assertion compares the whole row. + + // Non-nullable fixed-width underlying: no null-marker byte precedes the value. + {"Tuple(Int32, SimpleAggregateFunction(sum, UInt64), Float64)", + Arrays.asList(7, BigInteger.valueOf(42), 9.5d)}, + // Non-nullable variable-length underlying: still no marker. This is the contrast + // case - it would misalign if the SAF branch unconditionally wrote a marker. + {"Tuple(Int32, SimpleAggregateFunction(anyLast, String), Float64)", + Arrays.asList(7, "kept", 9.5d)}, + // Nullable underlying, value present: a single present-marker (0x00) precedes it. + {"Tuple(Int32, SimpleAggregateFunction(anyLast, Nullable(String)), Float64)", + Arrays.asList(7, "opt", 9.5d)}, + // Nullable underlying, value null: a single null-marker (0x01) and no value. + {"Tuple(Int32, SimpleAggregateFunction(anyLast, Nullable(String)), Float64)", + Arrays.asList(7, null, 9.5d)}, + }; + } + // Normalizes Tuple (Object[]) and Array (ArrayValue / List) results to nested Lists so // round-tripped values compare structurally regardless of the container representation the // reader returns. diff --git a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java index 1a0ee2287..d42891f0d 100644 --- a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java @@ -611,19 +611,23 @@ public void writeAggregateFunctionTests() throws Exception { } - //TODO: Do we support this? - @Test (groups = { "integration" }, enabled = false) + @Test (groups = { "integration" }) public void writeSimpleAggregateFunctionTests() throws Exception { String tableName = "rowBinaryFormatWriterTest_writeSimpleAggregateFunctionTests_" + UUID.randomUUID().toString().replace('-', '_'); String tableCreate = "CREATE TABLE \"" + tableName + "\" " + " (id Int32, " + - " simple_aggregate_function SimpleAggregateFunction(count, Int8), " + - " ) Engine = MergeTree ORDER BY id"; + " saf_sum SimpleAggregateFunction(sum, UInt64), " + + " saf_str SimpleAggregateFunction(anyLast, Nullable(String)), " + + " tail Int32 " + + " ) Engine = AggregatingMergeTree ORDER BY id"; - // Insert random (valid) values + // The SimpleAggregateFunction columns sit between id and a trailing Int32 so a byte + // dropped/added while writing them misaligns "tail" and is detected. Field[][] rows = new Field[][] {{ - new Field("id", 1), //Row ID - new Field("simple_aggregate_function", Arrays.asList((byte) 1)).set(Arrays.asList((byte) 1)), //SimpleAggregateFunction + new Field("id", 1), + new Field("saf_sum", BigInteger.valueOf(42)), + new Field("saf_str", "hello"), + new Field("tail", 7), } }; From 159f43a92c30c68ec792db26af7b15a8ae8e2dde Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Sun, 19 Jul 2026 12:40:47 +0000 Subject: [PATCH 2/3] Address review: clarify serializeNestedData SAF exception, fix CHANGELOG code spans - SerializerUtils: expand the top-level SimpleAggregateFunction case comment and serializeNestedData's Javadoc to note the deliberate exception to the "nested elements only" contract (the SAF wrapper is non-nullable so writeValuePreamble writes no marker, but a Nullable underlying still needs one). - CHANGELOG: reflow the SimpleAggregateFunction entry so no inline-code span wraps across a line break (renders correctly in Markdown), matching the file's other entries. Both changes are documentation-only; no behavior change. --- CHANGELOG.md | 11 ++++++----- .../api/data_formats/internal/SerializerUtils.java | 12 +++++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca7c339fb..1465b7b87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,12 @@ ### Bug Fixes -- **[client-v2]** Fixed the `RowBinary` writer throwing `UnsupportedOperationException: Unsupported data type: - SimpleAggregateFunction` when inserting into a `SimpleAggregateFunction(func, T)` column (the reader already - supported these columns). The value is now serialized identically to its underlying type `T`, writing the - `Nullable` null-marker byte when the underlying type is nullable (e.g. `SimpleAggregateFunction(anyLast, - Nullable(String))`), mirroring the read path. (https://github.com/ClickHouse/clickhouse-java/issues/2477) +- **[client-v2]** Fixed the `RowBinary` writer throwing + `UnsupportedOperationException: Unsupported data type: SimpleAggregateFunction` when inserting into a + `SimpleAggregateFunction(func, T)` column (the reader already supported these columns). The value is now + serialized identically to its underlying type `T`, writing the `Nullable` null-marker byte when the + underlying type is nullable (e.g. `SimpleAggregateFunction(anyLast, Nullable(String))`), mirroring the + read path. (https://github.com/ClickHouse/clickhouse-java/issues/2477) - **[client-v2]** Fixed binary array decoding for nullable element types so `Array(Nullable(Float64))` and similar columns now return boxed arrays such as `Double[]` instead of `Object[]`. This keeps null-supporting arrays aligned with their element type while preserving the existing `Object[]` fallback for Variant/Dynamic/Geometry arrays. (https://github.com/ClickHouse/clickhouse-java/issues/2846) 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 73d08198a..618b6b694 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 @@ -74,8 +74,11 @@ public static void serializeData(OutputStream stream, Object value, ClickHouseCo break; case SimpleAggregateFunction: // A SimpleAggregateFunction(func, T) value serializes identically to its underlying - // type T; delegate through serializeNestedData so a Nullable underlying still gets - // its RowBinary null-marker byte, mirroring BinaryStreamReader's read path. + // type T. This is a deliberate exception to serializeNestedData's "nested elements + // only" contract: the SAF wrapper is itself non-nullable, so writeValuePreamble emits + // no null-marker for a top-level SAF column, yet a Nullable underlying still needs one. + // serializeNestedData writes that marker iff the underlying is nullable, mirroring + // BinaryStreamReader's read path. serializeNestedData(stream, value, column.getNestedColumns().get(0)); break; case AggregateFunction: @@ -123,7 +126,10 @@ public static void serializeData(OutputStream stream, Object value, ClickHouseCo * {@code 0x01} when null), as the server expects for {@code Nullable} sub-columns in * {@code RowBinary}. For a top-level column this marker is instead written by * {@link com.clickhouse.client.api.data_formats.RowBinaryFormatSerializer#writeValuePreamble}, - * so this helper must only be used for nested elements. + * so this helper is normally used only for nested elements. The one deliberate exception is a + * top-level {@code SimpleAggregateFunction} column: its wrapper is itself non-nullable (so + * {@code writeValuePreamble} writes no marker), but its underlying type may be {@code Nullable} + * and still needs the marker, which this helper supplies. */ private static void serializeNestedData(OutputStream stream, Object value, ClickHouseColumn column) throws IOException { if (column.isNullable()) { From c6e7d068e01a2dd007b23fe49937f324d7d1175f Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:35:49 +0000 Subject: [PATCH 3/3] Fix SAF integration test DDL for CH 26.7+: use MergeTree writeSimpleAggregateFunctionTests created its test table with Engine = AggregatingMergeTree and a non-key `tail Int32` guard column. ClickHouse 26.7 tightened AggregatingMergeTree validation to reject non-key columns that are neither part of the sorting key nor aggregate measures (Code: 36 BAD_ARGUMENTS), breaking the build on CH latest while 25.3 / 25.8 / 26.5 still accepted it. Switch the engine to MergeTree: the SimpleAggregateFunction columns and the trailing `tail` misalignment guard remain valid on every server version, and DESCRIBE still reports the SimpleAggregateFunction types so the RowBinary writer path under test is unchanged. Matches the sibling integration tests in this file, which all use MergeTree. --- .../clickhouse/client/datatypes/RowBinaryFormatWriterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java index 00fd8d7c0..1763620c8 100644 --- a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java @@ -847,7 +847,7 @@ public void writeSimpleAggregateFunctionTests() throws Exception { " saf_sum SimpleAggregateFunction(sum, UInt64), " + " saf_str SimpleAggregateFunction(anyLast, Nullable(String)), " + " tail Int32 " + - " ) Engine = AggregatingMergeTree ORDER BY id"; + " ) Engine = MergeTree ORDER BY id"; // The SimpleAggregateFunction columns sit between id and a trailing Int32 so a byte // dropped/added while writing them misaligns "tail" and is detected.