Skip to content

Commit 0da71d8

Browse files
Fix client-v2 RowBinary: write Nullable marker for nested Tuple/Map elements
A Nullable element nested inside a Tuple or Map was serialized without its leading null-marker byte for present (non-null) values, corrupting the RowBinary stream so the server (and the client's own reader) mis-parsed the following bytes. Top-level columns get the marker from writeValuePreamble and serializeArrayData already wrote per-element markers, but serializeTupleData and serializeMapData called serializeData directly and skipped it. Add a serializeNestedData helper that writes the 0x00 (present) / 0x01 (null) marker for nested Nullable elements, and route Tuple elements and Map values through it. The array and top-level paths are unchanged. Fixes: #2721
1 parent 51c7955 commit 0da71d8

2 files changed

Lines changed: 92 additions & 3 deletions

File tree

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,25 @@ public static void serializeData(OutputStream stream, Object value, ClickHouseCo
111111
}
112112
}
113113

114+
/**
115+
* Serializes a value that is nested inside a container ({@code Tuple} or {@code Map}). A nested
116+
* {@code Nullable} element is prefixed with its null-marker byte ({@code 0x00} when present,
117+
* {@code 0x01} when null), as the server expects for {@code Nullable} sub-columns in
118+
* {@code RowBinary}. For a top-level column this marker is instead written by
119+
* {@link com.clickhouse.client.api.data_formats.RowBinaryFormatSerializer#writeValuePreamble},
120+
* so this helper must only be used for nested elements.
121+
*/
122+
private static void serializeNestedData(OutputStream stream, Object value, ClickHouseColumn column) throws IOException {
123+
if (column.isNullable()) {
124+
if (value == null) {
125+
writeNull(stream);
126+
return;
127+
}
128+
writeNonNull(stream);
129+
}
130+
serializeData(stream, value, column);
131+
}
132+
114133
private static final Map<Class<?>, ClickHouseColumn> PREDEFINED_TYPE_COLUMNS = getPredefinedTypeColumnsMap();
115134

116135
private static Map<Class<?>, ClickHouseColumn> getPredefinedTypeColumnsMap() {
@@ -460,12 +479,12 @@ public static void serializeTupleData(OutputStream stream, Object value, ClickHo
460479
if (value instanceof List) {
461480
List<?> values = (List<?>) value;
462481
for (int i = 0; i < values.size(); i++) {
463-
serializeData(stream, values.get(i), column.getNestedColumns().get(i));
482+
serializeNestedData(stream, values.get(i), column.getNestedColumns().get(i));
464483
}
465484
} else if (value.getClass().isArray()) {
466485
// TODO: this code uses reflection - we might need to measure it and find faster solution.
467486
for (int i = 0; i < Array.getLength(value); i++) {
468-
serializeData(stream, Array.get(value, i), column.getNestedColumns().get(i));
487+
serializeNestedData(stream, Array.get(value, i), column.getNestedColumns().get(i));
469488
}
470489
} else {
471490
throw new IllegalArgumentException("Cannot serialize " + value + " as a tuple");
@@ -483,7 +502,7 @@ public static void serializeMapData(OutputStream stream, Object value, ClickHous
483502
map.forEach((key, val) -> {
484503
try {
485504
serializePrimitiveData(stream, key, Objects.requireNonNull(column.getKeyInfo()));
486-
serializeData(stream, val, Objects.requireNonNull(column.getValueInfo()));
505+
serializeNestedData(stream, val, Objects.requireNonNull(column.getValueInfo()));
487506
} catch (IOException e) {
488507
throw new RuntimeException(e);
489508
}

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
import java.io.ByteArrayInputStream;
1010
import java.io.ByteArrayOutputStream;
1111
import java.util.Arrays;
12+
import java.util.LinkedHashMap;
1213
import java.util.List;
14+
import java.util.Map;
1315
import java.util.TimeZone;
1416

1517
@Test(groups = {"unit"})
@@ -134,6 +136,74 @@ public void testGeometrySerializationRejectsMalformedList() {
134136
ClickHouseColumn.of("v", "Geometry")));
135137
}
136138

139+
@Test
140+
public void testTupleWithNullableElementsRoundTrip() throws Exception {
141+
// Regression for #2721: a Nullable element nested inside a Tuple must be prefixed with its
142+
// null-marker byte (0x00 present, 0x01 null). Before the fix the present-value marker was
143+
// missing, so the reader mis-parsed the bytes that followed. The non-nullable sibling element
144+
// must keep being written without a marker.
145+
ClickHouseColumn column = ClickHouseColumn.of("v", "Tuple(Nullable(String), String)");
146+
147+
ByteArrayOutputStream present = new ByteArrayOutputStream();
148+
SerializerUtils.serializeData(present, Arrays.asList("optional-value", "value-2"), column);
149+
Assert.assertEquals((Object[]) newReader(present.toByteArray()).readValue(column),
150+
new Object[] {"optional-value", "value-2"});
151+
152+
ByteArrayOutputStream absent = new ByteArrayOutputStream();
153+
SerializerUtils.serializeData(absent, Arrays.asList(null, "value-2"), column);
154+
Assert.assertEquals((Object[]) newReader(absent.toByteArray()).readValue(column),
155+
new Object[] {null, "value-2"});
156+
}
157+
158+
@Test
159+
public void testMapWithNullableValuesRoundTrip() throws Exception {
160+
// Regression for #2721: a Nullable Map value must be prefixed with its null-marker byte.
161+
ClickHouseColumn column = ClickHouseColumn.of("v", "Map(String, Nullable(String))");
162+
Map<String, String> value = new LinkedHashMap<>();
163+
value.put("k1", "v1");
164+
value.put("k2", null);
165+
166+
ByteArrayOutputStream out = new ByteArrayOutputStream();
167+
SerializerUtils.serializeData(out, value, column);
168+
169+
Map<?, ?> result = newReader(out.toByteArray()).readValue(column);
170+
Assert.assertEquals(result, value);
171+
}
172+
173+
@Test
174+
public void testTupleWithNullableFixedWidthElementsRoundTrip() throws Exception {
175+
// #2721 is about the marker byte, not the element type: a fixed-width Nullable element
176+
// (here Int32) must also get its marker so the following value bytes are not misaligned.
177+
// Also covers an all-null tuple.
178+
ClickHouseColumn column = ClickHouseColumn.of("v", "Tuple(Nullable(Int32), Nullable(String))");
179+
180+
ByteArrayOutputStream present = new ByteArrayOutputStream();
181+
SerializerUtils.serializeData(present, Arrays.asList(42, "x"), column);
182+
Assert.assertEquals((Object[]) newReader(present.toByteArray()).readValue(column),
183+
new Object[] {42, "x"});
184+
185+
ByteArrayOutputStream allNull = new ByteArrayOutputStream();
186+
SerializerUtils.serializeData(allNull, Arrays.asList(null, null), column);
187+
Assert.assertEquals((Object[]) newReader(allNull.toByteArray()).readValue(column),
188+
new Object[] {null, null});
189+
}
190+
191+
@Test
192+
public void testNestedContainerWithNullableRoundTrip() throws Exception {
193+
// #2721: the marker handling must compose through nested containers — here a Map carrying a
194+
// Nullable value sits inside a Tuple, exercising serializeTupleData -> serializeMapData.
195+
ClickHouseColumn column = ClickHouseColumn.of("v", "Tuple(String, Map(String, Nullable(String)))");
196+
Map<String, String> inner = new LinkedHashMap<>();
197+
inner.put("k1", "v1");
198+
inner.put("k2", null);
199+
200+
ByteArrayOutputStream out = new ByteArrayOutputStream();
201+
SerializerUtils.serializeData(out, Arrays.asList("id", inner), column);
202+
203+
Assert.assertEquals((Object[]) newReader(out.toByteArray()).readValue(column),
204+
new Object[] {"id", inner});
205+
}
206+
137207
private void assertCustomGeoTypeTag(String typeName) throws Exception {
138208
ByteArrayOutputStream out = new ByteArrayOutputStream();
139209
SerializerUtils.writeDynamicTypeTag(out, ClickHouseColumn.of("v", typeName));

0 commit comments

Comments
 (0)