Skip to content

[client-v2] Fix reading Array(Nullable(T)) when values are read as another class #3293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
payallenka wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[client-v2] Fix reading Array(Nullable(T)) when values are read as another class #3293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2bdba37
[client-v2] Type nullable arrays by the values read
payallenka Nov 4, 2026
4bacdc3
[client-v2] Keep Array(Nullable(Point)) readable
payallenka Nov 6, 2026
f3ac350
Merge branch 'main' into fix/3283-nullable-array-item-type
Flipmonster Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions ...-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,10 +801,26 @@ public ArrayValue readArrayItem(ClickHouseColumn itemTypeColumn, int len) throws
}
ArrayValue array;
if (itemTypeColumn.isNullable()) {
Class<?> itemClass = resolveArrayItemClass(itemTypeColumn);
// Type the array by the values actually read: the declared type does not always tell
// which class readValue returns (UInt64 -> BigInteger, DateTime64 -> ZonedDateTime).
Object[] values = new Object[len];

@polyglotAI-bot polyglotAI-bot Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Nit: Extra Object[] buffer and copy for each nullable array

Each Array(Nullable(T)) value now allocates a temporary Object[len] in addition to the typed array, and copies it. docs/ai-review.md lists row decoding as performance-sensitive, and a large array doubles its transient memory.

With the approach in the comment on line 812, the reader can allocate the declared type, set each value directly, and copy once only when a value does not fit. To keep that copy rare, correct the remaining resolveArrayItemClass entries that disagree with readValue (for example Interval*, declared Long but read as Period / Duration).

A reply here is read on the next review: an explanation that holds up drops the finding.

Class<?> itemClass = null;
for (int i = 0; i < len; i++) {
Object value = readValue(itemTypeColumn);

@polyglotAI-bot polyglotAI-bot Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Major: Nullable(String) array elements become StringValue

readValue(itemTypeColumn) applies the top-level binary_string_support flag, so with the flag on, each Nullable(String) / Nullable(FixedString) element is a StringValue. The non-nullable branch uses readNestedValue, and the documented rule is that strings nested in containers stay String (constructor Javadoc, testJsonStringPathsStayStringWithBinaryStringSupport).

On main this combination throws (StringValue does not fit in String[]). With this change it returns List<StringValue> from getList for Array(Nullable(String)), while Array(String) returns List<String>. After a release, callers can depend on this, and a later fix becomes a behavior change.

readNestedValue still reads the null marker, so this one-line change keeps the branch correct:

Suggested change
Object value = readValue(itemTypeColumn);
Object value = readNestedValue(itemTypeColumn);

Please also add a test that reads Array(Nullable(String)) with binary_string_support set and expects String elements.

A reply here is read on the next review: an explanation that holds up drops the finding.

values[i] = value;
if (value != null) {
itemClass = itemClass == null ? value.getClass() : commonSuperclass(itemClass, value.getClass());

@polyglotAI-bot polyglotAI-bot Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Nit: Array component type changes and can differ per row

value.getClass() gives a component type that can differ from main and from row to row:

  • Array(Nullable(IPv6)): Inet4Address[], Inet6Address[] or InetAddress[], depending on the values in the row.
  • Types that already work: Ring / LineString / MultiPoint go from Object[] to Object[][], and JSON from Object[] to HashMap[]. An empty JSON value is Collections.emptyMap(), so this type can also differ per row.

This is visible through the public ArrayValue.getArray() and readValue(column, X[].class). getList, getObjectArray and JDBC getArray are not affected.

Suggestion: keep the resolveArrayItemClass corrections, start from resolveArrayItemClass(itemTypeColumn), and widen only when a value is not an instance of that class. Then every input that works on main keeps its component type, and only inputs that fail on main change. commonSuperclass must then accept an interface as the start class (its getSuperclass() is null), for example by returning Object.

A reply here is read on the next review: an explanation that holds up drops the finding.

}
}
if (itemClass == null) {
itemClass = resolveArrayItemClass(itemTypeColumn);
} else if (itemClass.isArray() && itemClass.getComponentType().isPrimitive()) {
// ArrayValue allocates Object[][] for an array class, which cannot hold Point's double[]
itemClass = Object.class;
}
array = new ArrayValue(itemClass, len);
for (int i = 0; i < len; i++) {
array.set(i, readValue(itemTypeColumn));
array.set(i, values[i]);
Comment thread
ccoltx marked this conversation as resolved.
Resolved
}
} else if (itemTypeColumn.getDataType() == ClickHouseDataType.Variant
|| itemTypeColumn.getDataType() == ClickHouseDataType.Dynamic
Expand Down Expand Up @@ -868,8 +884,15 @@ private static Class<?> resolveArrayItemClass(ClickHouseColumn itemTypeColumn) {
case UInt8:
case UInt16:
case UInt32:
case UInt64:
return dataType.getWiderObjectClass();
case UInt64:
return BigInteger.class;
case DateTime:
case DateTime32:
case DateTime64:
return ZonedDateTime.class;
case Tuple:
return Object[].class;
case Enum8:
case Enum16:
return EnumValue.class;
Expand Down Expand Up @@ -905,6 +928,18 @@ private static Class<?> resolveArrayItemClass(ClickHouseColumn itemTypeColumn) {
}
}

/**
* Returns the most specific class that both classes extend, so that an array of it can hold values
* of both. For example, IPv6 values can be read as {@link Inet4Address} or {@link Inet6Address}.
*/
private static Class<?> commonSuperclass(Class<?> a, Class<?> b) {
Class<?> cls = a;
while (!cls.isAssignableFrom(b)) {
cls = cls.getSuperclass();
}
return cls;
}

/**
* Reads a value nested inside a container (Array, Map, Tuple, Nested, Variant, JSON). Strings are always
* decoded into {@link String} here, regardless of the binary-string feature flag, because nested types
Expand Down
131 changes: 131 additions & 0 deletions ...com/clickhouse/client/api/data_formats/ClickHouseBinaryFormatReaderNullableArrayTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.InputStream;
import java.math.BigInteger;
import java.net.InetAddress;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.util.Arrays;
import java.util.List;
import java.util.TimeZone;

Expand Down Expand Up @@ -372,4 +377,130 @@ public void testNullableArraysWithMixedTypes() throws Exception {
Assert.assertNull(mixedList.get(3));
Assert.assertEquals(mixedList.get(4), 5L);
}

@Test
public void testNullableArraysOfTypesReadAsOtherClasses() throws Exception {
ByteArrayOutputStream out = new ByteArrayOutputStream();
String[] names = new String[]{"uint64_arr", "datetime64_arr", "ipv6_arr", "all_null_uint64_arr"};
String[] types = new String[]{"Array(Nullable(UInt64))", "Array(Nullable(DateTime64(3, 'UTC')))",
"Array(Nullable(IPv6))", "Array(Nullable(UInt64))"};

BinaryStreamUtils.writeVarInt(out, names.length);
for (String name : names) {
BinaryStreamUtils.writeString(out, name);
}
for (String type : types) {
BinaryStreamUtils.writeString(out, type);
}

// Array(Nullable(UInt64)): [0, NULL, 18446744073709551615]
BigInteger maxUInt64 = new BigInteger("18446744073709551615");
BinaryStreamUtils.writeVarInt(out, 3);
BinaryStreamUtils.writeNonNull(out);
BinaryStreamUtils.writeUnsignedInt64(out, BigInteger.ZERO);
BinaryStreamUtils.writeNull(out);
BinaryStreamUtils.writeNonNull(out);
BinaryStreamUtils.writeUnsignedInt64(out, maxUInt64);

// Array(Nullable(DateTime64(3, 'UTC'))): ['2026-01-01 13:34:56.780', NULL]
LocalDateTime dateTime = LocalDateTime.of(2026, 1, 1, 13, 34, 56, 780_000_000);
BinaryStreamUtils.writeVarInt(out, 2);
BinaryStreamUtils.writeNonNull(out);
BinaryStreamUtils.writeDateTime64(out, dateTime, 3, TimeZone.getTimeZone("UTC"));
BinaryStreamUtils.writeNull(out);

// Array(Nullable(IPv6)): ['::ffff:1.2.3.4', NULL, '2001:db8::1']
// An IPv4-mapped address is read as Inet4Address, the other one as Inet6Address.
BinaryStreamUtils.writeVarInt(out, 3);
BinaryStreamUtils.writeNonNull(out);
out.write(new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (byte) 0xff, (byte) 0xff, 1, 2, 3, 4});
BinaryStreamUtils.writeNull(out);
BinaryStreamUtils.writeNonNull(out);
out.write(InetAddress.getByName("2001:db8::1").getAddress());

// Array(Nullable(UInt64)): [NULL, NULL]
BinaryStreamUtils.writeVarInt(out, 2);
BinaryStreamUtils.writeNull(out);
BinaryStreamUtils.writeNull(out);

InputStream in = new ByteArrayInputStream(out.toByteArray());
QuerySettings querySettings = new QuerySettings().setUseTimeZone(TimeZone.getTimeZone("UTC").toZoneId().getId());
RowBinaryWithNamesAndTypesFormatReader reader =
new RowBinaryWithNamesAndTypesFormatReader(in, querySettings, new BinaryStreamReader.CachingByteBufferAllocator());

reader.next();

List<Object> uint64List = reader.getList("uint64_arr");
Assert.assertEquals(uint64List, Arrays.asList(BigInteger.ZERO, null, maxUInt64));

List<Object> dateTimeList = reader.getList("datetime64_arr");
Assert.assertEquals(dateTimeList, Arrays.asList(dateTime.atZone(ZoneId.of("UTC")), null));

List<Object> ipv6List = reader.getList("ipv6_arr");
Assert.assertEquals(ipv6List, Arrays.asList(InetAddress.getByName("1.2.3.4"), null,
InetAddress.getByName("2001:db8::1")));

List<Object> allNullList = reader.getList("all_null_uint64_arr");
Assert.assertEquals(allNullList, Arrays.asList(null, null));

@polyglotAI-bot polyglotAI-bot Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Nit: All-NULL assertion also passes on main

On main, an all-NULL Array(Nullable(UInt64)) is an UnsignedLong[] that holds nulls, so getList also returns [null, null] there. No test fails without the new UInt64 / DateTime* / Tuple entries in resolveArrayItemClass. Assert the component type, for example ((BinaryStreamReader.ArrayValue) reader.readValue("all_null_uint64_arr")).getArray().getClass(), or remove the entries.

Also:

A reply here is read on the next review: an explanation that holds up drops the finding.

}

@Test
public void testNullableArraysOfArrayBackedTypes() throws Exception {
ByteArrayOutputStream out = new ByteArrayOutputStream();
String[] names = new String[]{"point_arr", "ring_arr", "tuple_arr"};
String[] types = new String[]{"Array(Nullable(Point))", "Array(Nullable(Ring))",
"Array(Nullable(Tuple(Int32, Int32)))"};

BinaryStreamUtils.writeVarInt(out, names.length);
for (String name : names) {
BinaryStreamUtils.writeString(out, name);
}
for (String type : types) {
BinaryStreamUtils.writeString(out, type);
}

// Array(Nullable(Point)): [(1.5, 2.5), NULL]
BinaryStreamUtils.writeVarInt(out, 2);
BinaryStreamUtils.writeNonNull(out);
BinaryStreamUtils.writeFloat64(out, 1.5);
BinaryStreamUtils.writeFloat64(out, 2.5);
BinaryStreamUtils.writeNull(out);

// Array(Nullable(Ring)): [[(1.5, 2.5)], NULL]
BinaryStreamUtils.writeVarInt(out, 2);
BinaryStreamUtils.writeNonNull(out);
BinaryStreamUtils.writeVarInt(out, 1);
BinaryStreamUtils.writeFloat64(out, 1.5);
BinaryStreamUtils.writeFloat64(out, 2.5);
BinaryStreamUtils.writeNull(out);

// Array(Nullable(Tuple(Int32, Int32))): [(1, 2), NULL]
BinaryStreamUtils.writeVarInt(out, 2);
BinaryStreamUtils.writeNonNull(out);
BinaryStreamUtils.writeInt32(out, 1);
BinaryStreamUtils.writeInt32(out, 2);
BinaryStreamUtils.writeNull(out);

InputStream in = new ByteArrayInputStream(out.toByteArray());
QuerySettings querySettings = new QuerySettings().setUseTimeZone(TimeZone.getTimeZone("UTC").toZoneId().getId());
RowBinaryWithNamesAndTypesFormatReader reader =
new RowBinaryWithNamesAndTypesFormatReader(in, querySettings, new BinaryStreamReader.CachingByteBufferAllocator());

reader.next();

List<Object> points = reader.getList("point_arr");
Assert.assertEquals(points.size(), 2);
Assert.assertEquals((double[]) points.get(0), new double[]{1.5, 2.5});
Assert.assertNull(points.get(1));

List<Object> rings = reader.getList("ring_arr");
Assert.assertEquals(rings.size(), 2);
Assert.assertEquals(((double[][]) rings.get(0))[0], new double[]{1.5, 2.5});
Assert.assertNull(rings.get(1));

List<Object> tuples = reader.getList("tuple_arr");
Assert.assertEquals(tuples.size(), 2);
Assert.assertEquals((Object[]) tuples.get(0), new Object[]{1, 2});
Assert.assertNull(tuples.get(1));
}
}
8 changes: 8 additions & 0 deletions history/latest/3283.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
- **[client-v2]** Fixed reading `Array(Nullable(T))` columns whose element values are read as a different class than
the one declared for `T`. Reading failed with `IllegalArgumentException: array element type mismatch` for
`Array(Nullable(UInt64))` (values are `BigInteger`, the array was `UnsignedLong[]`), for
`Array(Nullable(DateTime64))` and `Array(Nullable(DateTime))` (values are `ZonedDateTime`, the array was
`LocalDateTime[]`), and for `Array(Nullable(IPv6))` holding IPv4-mapped addresses (read as `Inet4Address`). The
array is now typed by the values that were read. An array of only `NULL` elements gets the same type as one with
values for `UInt64` (`BigInteger`), `DateTime`/`DateTime64` (`ZonedDateTime`) and `Tuple` (`Object[]`).
(https://github.com/ClickHouse/clickhouse-java/issues/3283, https://github.com/ClickHouse/clickhouse-java/issues/3287)