[client-v2] Writing null into a non-nullable Array column produces a phantom extra row in RowBinary

[polyglotAI-bot]

Target

ClickHouse/clickhouse-java — module client-v2 (RowBinaryFormatWriter / RowBinaryFormatSerializer).

Summary

Writing a Java null into a non-nullable Array(...) column via RowBinaryFormatWriter serializes two bytes (00 00) where the correct RowBinary encoding of an empty array is a single byte (00). The server reads the extra 00 as a phantom extra row; in a multi-column row it misframes every following column and the insert fails with CANNOT_READ_ALL_DATA. This is silent data corruption, not an error.

This is distinct from #2931/#2932 (null → non-nullable Enum8/16, which was an NPE and is now fixed to throw). Here the defect is a double-write in the Array (and Dynamic) branch of writeValuePreamble.

Root cause

client-v2/.../data_formats/RowBinaryFormatSerializer.java, writeValuePreamble — non-defaults branch:

} else if (value == null) {                 // non-nullable column, Java null
    if (dataType == ClickHouseDataType.Array) {
        SerializerUtils.writeNonNull(out);  // emits 0x00  <-- extra byte
    } else if (dataType == ClickHouseDataType.Dynamic) {
        // do nothing
    } else {
        throw new IllegalArgumentException(
            String.format("An attempt to write null into not nullable column '%s'", column));
    }
}

writeValuePreamble returns true, so the caller then calls SerializerUtils.serializeData → serializeArrayData, which for a null value writes the var-uint length 0 (another 0x00):

// SerializerUtils.serializeArrayData
if (value == null) {
    writeVarInt(stream, 0);   // emits 0x00
    return;
}

Net wire output for null into a non-nullable Array: 00 00. Every other non-nullable type reaches the else and throws; Array/Dynamic are special-cased to let null through.

Empirical evidence (ClickHouse 26.5.1.882, raw RowBinary insert)

Single-column table CREATE TABLE t (a Array(Int32)) ENGINE=Memory:

raw bytes inserted	count()	SELECT a
00 (correct empty-array encoding)	1	[[]]
00 00 (what the client emits for null)	2	[[],[]] ← phantom row

Multi-column table CREATE TABLE t (a Array(Int32), b Int32), intending one row a=[] , b=7; the extra 00 shifts the frame:

Code: 33. DB::Exception: Cannot read all data. Bytes read: 0. Bytes expected: 4: (at row 2)
: While executing WaitForAsyncInsert. (CANNOT_READ_ALL_DATA)

Contrast (current, correct behaviors — must stay unchanged)

• Non-nullable Array + empty List/array [] → single 00 (correct).
• Nullable(Array(...)) + null → 01 null-marker (correct).
• Non-nullable Nested(...) + null → throws IllegalArgumentException: An attempt to write null into not nullable column (the safe behavior).

Scope to address (all four sites)

1. non-defaults Array — the corrupting double-write above.
2. non-defaults Dynamic — preamble does nothing, then serializeData(null, Dynamic) runs valueToColumnForDynamicType(null); needs verification/handling.
3. RowBinaryWithDefaults (defaultsSupport) Array/Dynamic — same special-case, but here the leading writeNonNull is the "no-default" marker, so the current output is a valid 00 00 = "value present, empty array" (i.e. it currently coerces null→[] without corruption). Only fires when the column has no DDL default (a column with a default already correctly emits the "use-default" marker).

Fix direction — needs a maintainer decision

Both candidate behaviors are a change from today's silent corruption:

• Option 1 — throw IllegalArgumentException (same message/behavior as every other non-nullable type, including Nested, and the just-merged Enum fix fix(client-v2): clear error for null in non-nullable Enum column #2932). Loudest, most consistent.
• Option 2 — treat null as an empty array (drop the extra writeNonNull in the non-defaults Array branch so only serializeArrayData's single var-uint 0 is written). Preserves the apparent original intent — the RowBinaryWithDefaults branch already coerces null→[] non-corruptingly, and arrays have a natural "empty" representation. Smaller, more permissive.

Recommendation: Option 1 (throw). #2931/#2932 posed the identical fork for Enum ("clear IllegalArgumentException" vs. "Python-style permissive / treat as default") and the merged fix (#2932, 2026-07-16) chose to throw. Applying the same choice to Array/Dynamic keeps null-into-non-nullable uniform across all types, and users retain explicit escape hatches (declare the column Nullable(...), pass [], or use RowBinaryWithDefaults with a DDL default). Option 2 is defensible but would make Array/Dynamic the only non-nullable types that silently accept null.

Whichever is chosen, it should be applied consistently to the Dynamic branch and the RowBinaryWithDefaults path (for the no-default case) so all four sites agree.

Origin

Found during work on PR #2936 (issue #2477, Nested RowBinary writer) as a separate, pre-existing defect on main. Filing so the fix direction can be decided before a PR is opened.

[alex-clickhouse]

@polyglotAI-bot fix this please