Summary
readVariant in @clickhouse/rowbinary (skills/clickhouse-js-node-rowbinary-parser/src/composite.ts) reads the 1-byte discriminant to pick which alternative reader to run, then returns only the decoded value and discards the discriminant. For most Variants the active alternative can be inferred from the JS value's shape, but for some it cannot — two alternatives can decode to an identical JS value yet correspond to different ClickHouse types, so a consumer cannot tell which one it got.
Why it matters
A consumer (e.g. a text serializer, or any user inspecting decoded values) needs the active type to format/interpret the value correctly. Verified against a live server (26.1):
| Variant |
value |
decoded JS (both alternatives) |
correct ClickHouse text |
Variant(UInt8, Enum8('a'=1,'b'=2)) |
1 |
number 1 (identical) |
1 (UInt8) vs a (Enum8) |
Variant(Date, DateTime) |
midnight |
Date(…T00:00:00) (identical) |
2020-01-02 vs 2020-01-02 00:00:00 |
In both rows the decoded value is indistinguishable, so without the discriminant there is no way to recover the right interpretation. (Cases like Variant(String, UInt64) or Variant(Int32, Int64) are unaffected — the shapes differ or the text is identical.)
These specific collision patterns (a numeric type beside an Enum, or Date beside DateTime/DateTime64) are rare — they don't appear in any of the upstream 0_stateless tests that use Variant — so this is a correctness/robustness fix for arbitrary user input, not a coverage blocker.
Proposed change
Have readVariant surface the active alternative index alongside the value, e.g.
// today
export function readVariant(readers): Reader<T[number] | null>
// proposed (shape TBD)
export function readVariant(readers): Reader<{ typeIndex: number; value: unknown } | null>
// (NULL discriminant 0xFF -> null, as today)
The reader already knows the index (it dispatches on the discriminant), so this is just threading it through. Consumers that don't care can ignore typeIndex.
Notes / considerations
- Breaking change to
readVariant's return type — should land with a minor/major bump and a changelog note.
- The type-AST → reader fold (
astToReader in compile.ts) maps the discriminant index to node.arguments[typeIndex]; a renderer can then format with the exact subtype.
- Discriminant order: alternatives are sorted by type name (ClickHouse's global ordering), which matches the header's argument order — see the existing note in
variantReader (compile.ts).
Found while scoping a Variant renderer for the upstream-SQL test harness (tests/clickhouse-test-runner), where it surfaced as the one case a value-only decode can't render correctly.
Summary
readVariantin@clickhouse/rowbinary(skills/clickhouse-js-node-rowbinary-parser/src/composite.ts) reads the 1-byte discriminant to pick which alternative reader to run, then returns only the decoded value and discards the discriminant. For mostVariants the active alternative can be inferred from the JS value's shape, but for some it cannot — two alternatives can decode to an identical JS value yet correspond to different ClickHouse types, so a consumer cannot tell which one it got.Why it matters
A consumer (e.g. a text serializer, or any user inspecting decoded values) needs the active type to format/interpret the value correctly. Verified against a live server (26.1):
Variant(UInt8, Enum8('a'=1,'b'=2))1number 1(identical)1(UInt8) vsa(Enum8)Variant(Date, DateTime)Date(…T00:00:00)(identical)2020-01-02vs2020-01-02 00:00:00In both rows the decoded value is indistinguishable, so without the discriminant there is no way to recover the right interpretation. (Cases like
Variant(String, UInt64)orVariant(Int32, Int64)are unaffected — the shapes differ or the text is identical.)These specific collision patterns (a numeric type beside an
Enum, orDatebesideDateTime/DateTime64) are rare — they don't appear in any of the upstream0_statelesstests that useVariant— so this is a correctness/robustness fix for arbitrary user input, not a coverage blocker.Proposed change
Have
readVariantsurface the active alternative index alongside the value, e.g.The reader already knows the index (it dispatches on the discriminant), so this is just threading it through. Consumers that don't care can ignore
typeIndex.Notes / considerations
readVariant's return type — should land with a minor/major bump and a changelog note.astToReaderincompile.ts) maps the discriminant index tonode.arguments[typeIndex]; a renderer can then format with the exact subtype.variantReader(compile.ts).Found while scoping a
Variantrenderer for the upstream-SQL test harness (tests/clickhouse-test-runner), where it surfaced as the one case a value-only decode can't render correctly.