Skip to content

Commit 9209449

Browse files
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: #2477
1 parent 565c59b commit 9209449

4 files changed

Lines changed: 64 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44

55
### Bug Fixes
66

7+
- **[client-v2]** Fixed the `RowBinary` writer throwing `UnsupportedOperationException: Unsupported data type:
8+
SimpleAggregateFunction` when inserting into a `SimpleAggregateFunction(func, T)` column (the reader already
9+
supported these columns). The value is now serialized identically to its underlying type `T`, writing the
10+
`Nullable` null-marker byte when the underlying type is nullable (e.g. `SimpleAggregateFunction(anyLast,
11+
Nullable(String))`), mirroring the read path. (https://github.com/ClickHouse/clickhouse-java/issues/2477)
12+
713
- **[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)
814

915
- **[client-v2]** Fixed `Float32`/`Float64` columns throwing `ClassCastException` when a value of a

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public static void serializeData(OutputStream stream, Object value, ClickHouseCo
7272
case Map:
7373
serializeMapData(stream, value, column);
7474
break;
75+
case SimpleAggregateFunction:
76+
// A SimpleAggregateFunction(func, T) value serializes identically to its underlying
77+
// type T; delegate through serializeNestedData so a Nullable underlying still gets
78+
// its RowBinary null-marker byte, mirroring BinaryStreamReader's read path.
79+
serializeNestedData(stream, value, column.getNestedColumns().get(0));
80+
break;
7581
case AggregateFunction:
7682
serializeAggregateFunction(stream, value, column);
7783
break;

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
@@ -217,6 +217,47 @@ private Object[][] nestedNullableData() throws Exception {
217217
};
218218
}
219219

220+
@Test(dataProvider = "simpleAggregateFunctionData")
221+
public void testSimpleAggregateFunctionRoundTrip(String typeName, Object value) throws Exception {
222+
ClickHouseColumn column = ClickHouseColumn.of("v", typeName);
223+
224+
ByteArrayOutputStream out = new ByteArrayOutputStream();
225+
SerializerUtils.serializeData(out, value, column);
226+
227+
Object actual = newReader(out.toByteArray()).readValue(column);
228+
Assert.assertEquals(normalize(actual), normalize(value));
229+
}
230+
231+
@DataProvider(name = "simpleAggregateFunctionData")
232+
private Object[][] simpleAggregateFunctionData() {
233+
return new Object[][] {
234+
// Top-level SAF columns - the exact shape reported in the bug, reached directly
235+
// through the serializeData switch's SimpleAggregateFunction case.
236+
{"SimpleAggregateFunction(sum, UInt64)", BigInteger.valueOf(42)},
237+
{"SimpleAggregateFunction(anyLast, Nullable(String))", "present"},
238+
239+
// A SimpleAggregateFunction(func, T) value serializes byte-identically to its
240+
// underlying type T. Each SAF below sits in the MIDDLE of the schema between a
241+
// leading Int32 and a trailing Float64, so a dropped or extra byte (such as a
242+
// wrongly written null-marker) shifts the trailing Float64 and is detected
243+
// positionally. The assertion compares the whole row.
244+
245+
// Non-nullable fixed-width underlying: no null-marker byte precedes the value.
246+
{"Tuple(Int32, SimpleAggregateFunction(sum, UInt64), Float64)",
247+
Arrays.asList(7, BigInteger.valueOf(42), 9.5d)},
248+
// Non-nullable variable-length underlying: still no marker. This is the contrast
249+
// case - it would misalign if the SAF branch unconditionally wrote a marker.
250+
{"Tuple(Int32, SimpleAggregateFunction(anyLast, String), Float64)",
251+
Arrays.asList(7, "kept", 9.5d)},
252+
// Nullable underlying, value present: a single present-marker (0x00) precedes it.
253+
{"Tuple(Int32, SimpleAggregateFunction(anyLast, Nullable(String)), Float64)",
254+
Arrays.asList(7, "opt", 9.5d)},
255+
// Nullable underlying, value null: a single null-marker (0x01) and no value.
256+
{"Tuple(Int32, SimpleAggregateFunction(anyLast, Nullable(String)), Float64)",
257+
Arrays.asList(7, null, 9.5d)},
258+
};
259+
}
260+
220261
// Normalizes Tuple (Object[]) and Array (ArrayValue / List) results to nested Lists so
221262
// round-tripped values compare structurally regardless of the container representation the
222263
// reader returns.

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -611,19 +611,23 @@ public void writeAggregateFunctionTests() throws Exception {
611611
}
612612

613613

614-
//TODO: Do we support this?
615-
@Test (groups = { "integration" }, enabled = false)
614+
@Test (groups = { "integration" })
616615
public void writeSimpleAggregateFunctionTests() throws Exception {
617616
String tableName = "rowBinaryFormatWriterTest_writeSimpleAggregateFunctionTests_" + UUID.randomUUID().toString().replace('-', '_');
618617
String tableCreate = "CREATE TABLE \"" + tableName + "\" " +
619618
" (id Int32, " +
620-
" simple_aggregate_function SimpleAggregateFunction(count, Int8), " +
621-
" ) Engine = MergeTree ORDER BY id";
619+
" saf_sum SimpleAggregateFunction(sum, UInt64), " +
620+
" saf_str SimpleAggregateFunction(anyLast, Nullable(String)), " +
621+
" tail Int32 " +
622+
" ) Engine = AggregatingMergeTree ORDER BY id";
622623

623-
// Insert random (valid) values
624+
// The SimpleAggregateFunction columns sit between id and a trailing Int32 so a byte
625+
// dropped/added while writing them misaligns "tail" and is detected.
624626
Field[][] rows = new Field[][] {{
625-
new Field("id", 1), //Row ID
626-
new Field("simple_aggregate_function", Arrays.asList((byte) 1)).set(Arrays.asList((byte) 1)), //SimpleAggregateFunction
627+
new Field("id", 1),
628+
new Field("saf_sum", BigInteger.valueOf(42)),
629+
new Field("saf_str", "hello"),
630+
new Field("tail", 7),
627631
}
628632
};
629633

0 commit comments

Comments
 (0)