Skip to content

Commit d907c94

Browse files
chernserchernser
authored
Merge pull request #2817 from ClickHouse/03/31/26/fix_null_values
Fix NULL handling for Variant/Dynamic
2 parents 11305dd + 70c0c33 commit d907c94

5 files changed

Lines changed: 142 additions & 4 deletions

File tree

‎client-v2/src/main/java/com/clickhouse/client/api/data_formats/RowBinaryFormatSerializer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public static boolean writeValuePreamble(OutputStream out, boolean defaultsSuppo
207207
} else if (dataType == ClickHouseDataType.Array) {//If the column is an array
208208
SerializerUtils.writeNonNull(out);//Then we send nonNull
209209
} else if (dataType == ClickHouseDataType.Dynamic) {
210-
// do nothing
210+
SerializerUtils.writeNonNull(out);
211211
} else {
212212
throw new IllegalArgumentException(String.format("An attempt to write null into not nullable column '%s'", column));
213213
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,10 @@ public ArrayValue readNested(ClickHouseColumn column) throws IOException {
886886
}
887887

888888
public Object readVariant(ClickHouseColumn column) throws IOException {
889-
int ordNum = readByte();
889+
int ordNum = readByte() & 0xFF;
890+
if (ordNum == 0xFF) {
891+
return null;
892+
}
890893
return readValue(column.getNestedColumns().get(ordNum));
891894
}
892895

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package com.clickhouse.client.api.data_formats.internal;
22

3+
import com.clickhouse.data.ClickHouseColumn;
4+
35
import java.io.ByteArrayInputStream;
46
import java.io.ByteArrayOutputStream;
57
import java.io.IOException;
6-
import java.lang.reflect.Array;
78
import java.time.ZoneId;
89
import java.time.ZonedDateTime;
910
import java.time.temporal.ChronoUnit;
10-
import java.util.Arrays;
1111
import java.util.TimeZone;
1212

1313
import org.testng.Assert;
@@ -190,4 +190,18 @@ public void testArrayValue() throws Exception {
190190
Object[] array2 = array.getArrayOfObjects();
191191
Assert.assertEquals(array1.length, array2.length);
192192
}
193+
194+
@Test
195+
public void testReadNullVariantReturnsNull() throws Exception {
196+
ClickHouseColumn column = ClickHouseColumn.of("v", "Variant(Int32, String)");
197+
BinaryStreamReader reader = new BinaryStreamReader(
198+
new ByteArrayInputStream(new byte[]{(byte) 0xFF}),
199+
TimeZone.getTimeZone("UTC"),
200+
null,
201+
new BinaryStreamReader.CachingByteBufferAllocator(),
202+
false,
203+
null);
204+
205+
Assert.assertNull(reader.readValue(column));
206+
}
193207
}

‎client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,14 @@ public static class DTOForVariantPrimitivesTests {
291291
private Object field;
292292
}
293293

294+
@Data
295+
@AllArgsConstructor
296+
@NoArgsConstructor
297+
public static class DTOForVariantNullPojoTests {
298+
private int rowId;
299+
private Object value;
300+
}
301+
294302
@Test(groups = {"integration"})
295303
public void testVariantWithDecimals() throws Exception {
296304
testVariantWith("decimals", new String[]{"field Variant(String, Decimal(4, 4))"},
@@ -313,6 +321,56 @@ public void testVariantWithDecimals() throws Exception {
313321
});
314322
}
315323

324+
@Test(groups = {"integration"})
325+
public void testVariantNullLiteral() throws Exception {
326+
if (isVersionMatch("(,24.8]")) {
327+
return;
328+
}
329+
330+
List<GenericRecord> records = client.queryAll(
331+
"SELECT NULL::Variant(Int32, String) AS val",
332+
new QuerySettings().serverSetting("allow_experimental_variant_type", "1"));
333+
334+
Assert.assertEquals(records.size(), 1);
335+
Assert.assertNull(records.get(0).getObject("val"));
336+
}
337+
338+
@Test(groups = {"integration"})
339+
public void testVariantNullLiteralWithPojo() throws Exception {
340+
if (isVersionMatch("(,24.8]")) {
341+
return;
342+
}
343+
344+
final String table = "test_variant_null_literal_pojo";
345+
CommandSettings createTableSettings = (CommandSettings) new CommandSettings()
346+
.serverSetting("allow_experimental_variant_type", "1");
347+
QuerySettings querySettings = new QuerySettings().serverSetting("allow_experimental_variant_type", "1");
348+
349+
client.execute("DROP TABLE IF EXISTS " + table).get();
350+
client.execute(tableDefinition(table,
351+
"rowId Int32",
352+
"value Variant(Int32, String)"), createTableSettings).get();
353+
354+
try (QueryResponse ignored = client.query(
355+
"INSERT INTO " + table
356+
+ " SELECT 1 AS rowId, NULL::Variant(Int32, String) AS value"
357+
+ " UNION ALL SELECT 2, 42::Variant(Int32, String)"
358+
+ " UNION ALL SELECT 3, 'hello'::Variant(Int32, String)",
359+
querySettings).get()) {
360+
}
361+
362+
TableSchema tableSchema = client.getTableSchema(table);
363+
client.register(DTOForVariantNullPojoTests.class, tableSchema);
364+
365+
List<DTOForVariantNullPojoTests> items =
366+
client.queryAll("SELECT * FROM " + table + " ORDER BY rowId", DTOForVariantNullPojoTests.class, tableSchema);
367+
368+
Assert.assertEquals(items, Arrays.asList(
369+
new DTOForVariantNullPojoTests(1, null),
370+
new DTOForVariantNullPojoTests(2, 42),
371+
new DTOForVariantNullPojoTests(3, "hello")));
372+
}
373+
316374
@Test(groups = {"integration"})
317375
public void testVariantWithArrays() throws Exception {
318376
testVariantWith("arrays", new String[]{"field Variant(String, Array(String))"},
@@ -811,13 +869,61 @@ public void testDynamicWithVariant() throws Exception {
811869
Assert.assertEquals(val, 3);
812870
}
813871

872+
@Test(groups = {"integration"})
873+
public void testDynamicNullWithDefaultsColumn() throws Exception {
874+
if (isVersionMatch("(,24.8]")) {
875+
return;
876+
}
877+
878+
final String table = "test_dynamic_null_defaults";
879+
CommandSettings createTableSettings = (CommandSettings) new CommandSettings()
880+
.serverSetting("allow_experimental_dynamic_type", "1");
881+
882+
client.execute("DROP TABLE IF EXISTS " + table).get();
883+
client.execute(tableDefinition(table,
884+
"rowId Int32",
885+
"dyn Dynamic",
886+
"extra String DEFAULT 'default_val'"), createTableSettings).get();
887+
888+
client.register(DTOForDynamicDefaultsTests.class, client.getTableSchema(table));
889+
890+
List<DTOForDynamicDefaultsTests> data = Arrays.asList(
891+
new DTOForDynamicDefaultsTests(1, "hello", "explicit"),
892+
new DTOForDynamicDefaultsTests(2, null, "after_null"),
893+
new DTOForDynamicDefaultsTests(3, 42L, "last"));
894+
client.insert(table, data).get().close();
895+
896+
List<GenericRecord> records = client.queryAll("SELECT rowId, dyn, extra FROM " + table + " ORDER BY rowId");
897+
Assert.assertEquals(records.size(), data.size());
898+
899+
Assert.assertEquals(records.get(0).getInteger("rowId"), 1);
900+
Assert.assertEquals(records.get(0).getString("dyn"), "hello");
901+
Assert.assertEquals(records.get(0).getString("extra"), "explicit");
902+
903+
Assert.assertEquals(records.get(1).getInteger("rowId"), 2);
904+
Assert.assertNull(records.get(1).getObject("dyn"));
905+
Assert.assertEquals(records.get(1).getString("extra"), "after_null");
906+
907+
Assert.assertEquals(records.get(2).getInteger("rowId"), 3);
908+
Assert.assertEquals(records.get(2).getString("dyn"), "42");
909+
Assert.assertEquals(records.get(2).getString("extra"), "last");
910+
}
911+
814912
@Data
815913
@AllArgsConstructor
816914
public static class DTOForDynamicPrimitivesTests {
817915
private int rowId;
818916
private Object field;
819917
}
820918

919+
@Data
920+
@AllArgsConstructor
921+
public static class DTOForDynamicDefaultsTests {
922+
private int rowId;
923+
private Object dyn;
924+
private String extra;
925+
}
926+
821927
@Test(groups = {"integration"})
822928
public void testAllDataTypesKnown() {
823929
List<GenericRecord> dbTypes = client.queryAll("SELECT * FROM system.data_type_families");

‎client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package com.clickhouse.client.internal;
22

3+
import com.clickhouse.client.api.data_formats.RowBinaryFormatSerializer;
34
import com.clickhouse.client.api.data_formats.internal.SerializerUtils;
5+
import com.clickhouse.data.ClickHouseColumn;
46
import org.testng.annotations.Test;
57

8+
import java.io.ByteArrayOutputStream;
9+
610
import org.testng.Assert;
711

812
public class SerializerUtilsTests {
@@ -13,4 +17,15 @@ public void testConvertToInteger() {
1317
Assert.assertEquals(SerializerUtils.convertToInteger("1640995199").intValue(), expected);
1418
Assert.assertEquals(SerializerUtils.convertToInteger(false).intValue(), 0);
1519
}
20+
21+
@Test
22+
public void testDynamicNullWithDefaultsWritesPreambleAndNothingTag() throws Exception {
23+
ClickHouseColumn column = ClickHouseColumn.of("dyn", "Dynamic");
24+
ByteArrayOutputStream out = new ByteArrayOutputStream();
25+
26+
Assert.assertTrue(RowBinaryFormatSerializer.writeValuePreamble(out, true, column, null));
27+
SerializerUtils.serializeData(out, null, column);
28+
29+
Assert.assertEquals(out.toByteArray(), new byte[]{0, 0});
30+
}
1631
}

0 commit comments

Comments
 (0)