[client-v2] serializeArrayData silently writes zero bytes for a wrong-typed Array/geo value and desynchronizes the RowBinary stream

[polyglotAI-bot]

Description

SerializerUtils.serializeArrayData writes zero bytes for a non-null value that is neither a Java array nor a List. The guard if (value instanceof List<?> || isArray) is false and there is no else branch, so the method returns having emitted nothing at all for the column — not even the var-int length 0.

In RowBinary this desynchronizes the stream: the bytes of the following column are consumed as this column's length prefix, so every subsequent column in the row is misframed. Depending on the format the user gets either a confusing server-side CANNOT_READ_ALL_DATA (Code 33) or — with RowBinaryWithDefaults — a silently successful insert that stores phantom, corrupted rows.

Six dispatch paths in serializeData reach serializeArrayData and are all affected:

• case Array → serializeArrayData directly;
• case Ring / LineString → unwrap ClickHouseGeoRingValue, then serializeArrayData(…, GEO_RING_ARRAY);
• case Polygon / MultiLineString → GEO_POLYGON_ARRAY;
• case MultiPolygon → GEO_MULTI_POLYGON_ARRAY.

For the geo cases the unwrap is conditional (value instanceof ClickHouseGeoXxxValue ? … : value), so a wrong-typed value stays un-unwrapped and lands in the silent-skip.

This is a misuse scenario (passing a scalar to an array-shaped column), so it is not high-frequency. What makes it worth fixing is the failure mode: silent stream corruption instead of a clear client-side error — and it is inconsistent with the sibling serializers, which all fail fast (see “Contrast” below). Whether you'd prefer this as a misuse guard or consider the current behavior acceptable is a maintainer call; the reproduction below is offered so that call can be made on evidence.

Steps to reproduce

1. CREATE TABLE t (id UInt32, val Array(String), tail String) ENGINE MergeTree ORDER BY id
2. Insert one row via RowBinaryFormatWriter, setting val to a String ("not-an-array") instead of a List/array, and tail to "TAILVALUE".
3. Observe: no client-side error; the server rejects the row with Code 33 (RowBinary), or the insert succeeds storing 4 corrupted rows (RowBinaryWithDefaults).

Error Log or Exception StackTrace

Direct byte-level evidence — serializeArrayData / serializeData into a ByteArrayOutputStream:

Array(String)   <- String  : bytes written = 0     <-- defect
Ring            <- String  : bytes written = 0     <-- defect
LineString      <- String  : bytes written = 0     <-- defect
Polygon         <- String  : bytes written = 0     <-- defect
MultiLineString <- String  : bytes written = 0     <-- defect
MultiPolygon    <- String  : bytes written = 0     <-- defect

Array(String)   <- null            : bytes written = 1   (var-int 0 — correct)
Array(String)   <- List["a","b"]   : bytes written = 5   (correct)

End-to-end through RowBinaryFormatWriter + client.insert(...) against a live server (26.5.1.882):

Array(String) <- String (RowBinary)
  => Code: 33. DB::Exception: Cannot read all data. Bytes read: 8. Bytes expected: 84:
     (at row 1) : While executing BinaryRowInputFormat. (CANNOT_READ_ALL_DATA)

Array(UInt32) <- Integer (RowBinary)
  => Code: 33. DB::Exception: Cannot read all data. Bytes read: 1. Bytes expected: 4: (at row 1) ...

Ring            <- String (RowBinary) => Code: 33 ... Bytes read: 1. Bytes expected: 8: (at row 1)
LineString      <- String (RowBinary) => Code: 33 ... Bytes read: 1. Bytes expected: 8: (at row 1)
Polygon         <- String (RowBinary) => Code: 33 ... Bytes read: 0. Bytes expected: 8: (at row 1)
MultiLineString <- String (RowBinary) => Code: 33 ... Bytes read: 0. Bytes expected: 8: (at row 1)
MultiPolygon    <- String (RowBinary) => Code: 33 ... Bytes read: 7. Bytes expected: 8: (at row 1)

The worst variant — the insert silently succeeds and stores garbage:

Array(String) <- String (RowBinaryWithDefaults)
  => INSERT SUCCEEDED writtenRows=4
     stored = [id=0, val=[], tail=] [id=0, val=[], tail=] [id=0, val=[], tail=] [id=1, val=[], tail=]

One committed row became four rows, the id and tail values were lost, and no exception was raised anywhere.

Expected Behaviour

A non-null value that cannot represent an array should be rejected client-side with a clear IllegalArgumentException naming the column and the offending value type — matching what the sibling serializers already do today.

Contrast (current, correct behaviors — verified in the same run, must stay unchanged):

Tuple(UInt32, String) <- String => IllegalArgumentException: Cannot serialize not-a-tuple as a tuple
Geometry              <- String => IllegalArgumentException: Cannot write value of class class java.lang.String
                                   into column with geometry type Geometry
Map(String, String)   <- String => ClassCastException: class java.lang.String cannot be cast to
                                   class java.util.Map
QBit(...)             <- String => IllegalArgumentException (guarded in #2939)
Array(String)         <- null          => single var-int 0
Array(String)         <- List["a","b"] => round-trips: stored val=['a','b'], tail=TAILVALUE

So Array and the five array-backed geo types are the only container paths that fail silently.

Code Example

// CREATE TABLE t (id UInt32, val Array(String), tail String) ENGINE MergeTree ORDER BY id
TableSchema schema = client.getTableSchema("t");
ClickHouseFormat format = ClickHouseFormat.RowBinary;      // or RowBinaryWithDefaults
client.insert("t", out -> {
    RowBinaryFormatWriter w = new RowBinaryFormatWriter(out, schema, format);
    w.setValue(schema.nameToColumnIndex("id"),   1);
    w.setValue(schema.nameToColumnIndex("val"),  "not-an-array");   // wrong type, silently skipped
    w.setValue(schema.nameToColumnIndex("tail"), "TAILVALUE");
    w.commitRow();
}, format, new InsertSettings()).get();

// RowBinary            -> server Code 33 CANNOT_READ_ALL_DATA
// RowBinaryWithDefaults-> insert succeeds, 4 phantom rows stored

Byte-level minimal case, no server needed:

ByteArrayOutputStream out = new ByteArrayOutputStream();
SerializerUtils.serializeArrayData(out, "not-an-array", ClickHouseColumn.of("arr", "Array(String)"));
assert out.size() == 0;   // currently true — nothing is written for the column

Root cause

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java, serializeArrayData (line 516 on main @ 601ade16):

public static void serializeArrayData(OutputStream stream, Object value, ClickHouseColumn column) throws IOException {
    if (value == null) {
        writeVarInt(stream, 0);
        return;
    }

    boolean isArray = value.getClass().isArray();
    if (value instanceof List<?> || isArray) {
        ...
    }
    // <-- no else: a non-null, non-array, non-List value falls off the end and writes 0 bytes
}

The dispatch sites are serializeData lines 70-71 (Array) and 105-118 (Ring/LineString/Polygon/MultiLineString/MultiPolygon).

Suggested fix

In serializeArrayData, after the null check, reject a value that is neither a Java array nor a List:

if (!(value instanceof List<?>) && !value.getClass().isArray()) {
    throw new IllegalArgumentException("Array column '" + column.getColumnName()
            + "' expects a Java array or List but got " + value.getClass().getName());
}

This mirrors the guards already present in serializerGeometry, serializeTupleData, and serializeQBitData (added in #2939 for exactly this failure class, scoped to the QBit path only). Because the throw sits in serializeArrayData itself, all six dispatch paths are covered at once. For the geo cases the message will name the synthetic GEO_*_ARRAY column, so it may be worth passing the real column name through or naming the geo type in the message.

Regression tests should assert that a wrong-typed value throws (and that zero bytes were written) for Array plus each of the five geo types, with contrast cases that a valid List/array still round-trips and that null still yields a single var-int 0.

Suggested labels: bug, client-api-v2, area:data-type, area:client-insert.

Configuration

Client Configuration

new Client.Builder()
    .addEndpoint(Protocol.HTTP, host, port, false)
    .setUsername("default").setPassword(...)
    .setDefaultDatabase(...)
    .serverSetting(ServerSettings.ASYNC_INSERT, "0")
    .serverSetting(ServerSettings.WAIT_END_OF_QUERY, "1")
    .build();

Environment

• Cloud
• Client version: main @ 601ade16 (0.10.0-rc1-SNAPSHOT)
• Language version: OpenJDK 17
• OS: Linux (x86_64, container)

ClickHouse Server

• ClickHouse Server version: 26.5.1.882 (official build)
• ClickHouse Server non-default settings, if any: none relevant (async_insert=0, wait_end_of_query=1 set per-query)
• CREATE TABLE statements for tables involved:

  CREATE TABLE t (id UInt32, val Array(String), tail String) ENGINE MergeTree ORDER BY id;
  -- and the same shape with val of type Ring / LineString / Polygon / MultiLineString / MultiPolygon

• Sample data: none needed — the defect is on the insert path.

Relationship to other issues

Distinct from #2938 / PR #2940, which cover the null-into-non-nullable-Array double-0x00 write in RowBinaryFormatSerializer.writeValuePreamble. That is a different site and a different root cause; this report is about a non-null wrong-typed value in serializeArrayData itself. The two are complementary and #2940 does not touch this path.

Origin

Found by automated analysis of client-v2 while working on the Cursor Bugbot review of PR #2939 (issue #2610) — the QBit guard added there fixed one dispatch path of this same mechanism, and this report covers the general case that was left out of that feature PR's scope. Verified by running against a live ClickHouse server (byte-level and end-to-end), not by inspection alone.