[client-v2] readVariant discards the active type discriminant — colliding Variant alternatives are indistinguishable

[claude]

Description

BinaryStreamReader.readVariant(ClickHouseColumn) reads the 1-byte Variant discriminant (ordNum) only to select which nested column's reader to run, then returns just the decoded value and throws the discriminant away:

// client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java:892
public Object readVariant(ClickHouseColumn column) throws IOException {
    int ordNum = readByte() & 0xFF;
    if (ordNum == 0xFF) {
        return null;
    }
    return readValue(column.getNestedColumns().get(ordNum)); // ordNum (the active type) is discarded
}

For many Variants the active alternative can be inferred from the returned object's runtime class, but for alternatives whose readers produce the same Java runtime type the information is lost, and the consumer cannot recover which ClickHouse subtype was actually on the wire. Examples where the decoded objects are indistinguishable:

Variant	both alternatives decode to	why ambiguous
Variant(DateTime, DateTime64(3))	java.time.ZonedDateTime (lines 186-191 → convertDateTime, typeHint == null → returns ZonedDateTime)	same class, same instant
Variant(String, FixedString(N))	java.lang.String	same class
Variant(Decimal32(s), Decimal64(s))	java.math.BigDecimal	same class/scale

Because readVariant returns a bare Object, a downstream consumer (text/JSON renderer, type-aware formatter, or any caller inspecting the value) has no way to choose the correct subtype for formatting in these cases.

This is the Java analogue of ClickHouse/clickhouse-js#910. Note the specific examples in that JS issue do not collide here — clickhouse-java maps Enum8/Enum16 to a dedicated EnumValue (lines 172-181) and Date to LocalDate (line 183), so Variant(UInt8, Enum8(...)) and Variant(Date, DateTime) are recoverable from the runtime type. The underlying limitation — the discriminant index is not surfaced — is the same, and other type pairs (above) hit it.

ClickHouse server version

26.6.1.1193 (collision behavior determined by code analysis of the type→class mapping; not exercised end-to-end against the server).

Reproduction

A Variant(DateTime, DateTime64(3)) column: whichever alternative the server picks, readVariant returns a ZonedDateTime, so the caller cannot tell DateTime from DateTime64.

// Conceptual unit-level repro against BinaryStreamReader
ClickHouseColumn col = ClickHouseColumn.of("v", "Variant(DateTime, DateTime64(3))");

// Wire bytes: discriminant 0x01 selects the DateTime64 alternative (alternatives are
// sorted by ClickHouse's global type-name ordering), followed by its encoded value.
byte[] payload = /* 0x01 ++ encoded DateTime64 value */;
BinaryStreamReader reader = new BinaryStreamReader(
        new ByteArrayInputStream(payload), null, LZ4_FACTORY, null);

Object value = reader.readVariant(col);

// EXPECTED: caller can determine the active alternative was DateTime64 (index 1)
// ACTUAL:   value is a java.time.ZonedDateTime, identical to what the DateTime
//           alternative (index 0) would have produced — the active type is unrecoverable.
assertTrue(value instanceof ZonedDateTime); // passes for BOTH alternatives

The same happens through the high-level reader path (readValue → case Variant at line 248-250): reading a Variant(String, FixedString(10)) column yields a String with no indication of which alternative it was.

Suggested fix

Surface the active alternative index alongside the value rather than discarding it — mirroring the upstream proposal. Options:

• Return a small wrapper, e.g. VariantValue { int typeIndex; Object value; }, from readVariant (breaking change to the public Object readVariant(...) signature — gate behind a minor/major bump), or
• Expose the resolved subtype (column.getNestedColumns().get(ordNum)) so a renderer can format with the exact ClickHouseColumn.

Buggy code: BinaryStreamReader.readVariant at client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java:892-898 (dispatched from readValue case Variant at line 248-250). The legacy clickhouse-data RowBinary processor should be checked for the same pattern.

Link

Relayed from ClickHouse/clickhouse-js#910.

[polyglotAI-bot]

Confirmed by source analysis — but the fix is a public-API design decision, not a localized patch. Flagging for maintainer direction before any PR, since every viable fix touches the public contract and there is no accepted cross-client design yet.

Confirmed

BinaryStreamReader.readVariant(...) (client-v2/.../data_formats/internal/BinaryStreamReader.java:892) reads the 1-byte discriminant only to select the nested column, then returns the bare decoded value:

public Object readVariant(ClickHouseColumn column) throws IOException {
    int ordNum = readByte() & 0xFF;
    if (ordNum == 0xFF) return null;
    return readValue(column.getNestedColumns().get(ordNum)); // ordNum discarded
}

The high-level reader materializes that bare value straight into the row — readValue(...) case Variant: -> readVariant(actualColumn) (lines 248-250) — so there is no per-cell channel to carry the active subtype.

All three reported collisions hold in the current type->class mapping:

Variant	both alternatives decode to	mapping
Variant(DateTime, DateTime64(3))	java.time.ZonedDateTime	lines 186-191 (convertDateTime)
Variant(Decimal32(s), Decimal64(s))	java.math.BigDecimal	lines 158-161 (readDecimal)
Variant(String, FixedString(N))	java.lang.String	lines 123-131

(As the issue notes, the JS examples Variant(UInt8, Enum8) / Variant(Date, DateTime) do not collide here, because Java maps Enum8/16 -> EnumValue and Date -> LocalDate.)

Why this isn't a small, backward-compatible fix

Surfacing the discriminant means changing what a Variant cell exposes to callers, and every option touches the public contract:

1. The colliding types can't be transparently wrapped. String and ZonedDateTime are final; subclassing BigDecimal would break equals / getClass()-based consumers. So "still a String, but also carries the active type" is not feasible.
2. Changing readVariant's return type (e.g. to a VariantValue { int typeIndex; Object value; }) changes the externally-visible output type of every Variant — and Geometry (line 249 reuses readVariant) — cell. That breaks existing callers doing (String) record.getObject("v").
3. A new opt-in API + wrapper type is non-breaking, but is a genuine public-API addition: naming, how a caller requests the type-aware form, and interaction with POJO binding / JDBC getObject / the type-mapping system all have to be decided.

docs/ai-review.md makes API/behavior stability the top priority, lists "return value types, wrapper types" under Breaking change review first, and says not to introduce such changes "unless the task explicitly requires." There is no single localized patch a maintainer would merge without first choosing the API shape — which is why I'm requesting direction rather than opening a speculative breaking PR.

This is a shared, design-level limitation, so the chosen shape should stay consistent across siblings: Geometry reuses readVariant and docs/features.md already documents the analogous Geometry ambiguity (Ring vs LineString, Polygon vs MultiLineString) as not currently distinguishable; Dynamic (readDynamicData(), line 1205) likewise returns a bare value with no surfaced concrete type.

Cross-client coordination

The same root cause is tracked in the sibling clients, and all three are still open with no accepted design:

• C#: VariantType.Read discards the active type discriminant, making colliding alternatives indistinguishable clickhouse-cs#388
• JS (original proposal): readVariant should return the active type discriminant, not just the value clickhouse-js#910

A consistent cross-client shape is worth deciding once.

Requesting direction

Which would you prefer for client-v2 (so the same shape can be mirrored in cs/js)?

• A — Breaking: readVariant (and the Variant/Geometry cell type) returns a small VariantValue { int activeTypeIndex; ClickHouseColumn activeType; Object value; }, gated behind a version bump.
• B — Additive: keep readVariant as-is; add an opt-in type-aware read path / accessor that surfaces column.getNestedColumns().get(ordNum) alongside the value.
• C — Something else.

Happy to implement whichever shape you choose.

— automated analysis by @polyglotAI-bot