TCP J3: Add Variant(...) support by alex-clickhouse · Pull Request #459 · ClickHouse/clickhouse-cs

alex-clickhouse · 2026-07-22T14:24:52Z

Stacked on #456 (TCP J1: LowCardinality). Adds Variant(...) support and the write-path prefix-signature change it sits on top of.

J0a — `WriteStatePrefix` receives the sliced column

Widened IColumnCodec.WriteStatePrefix(writer) → WriteStatePrefix(writer, column, start, length). A type whose prefix bytes derive from the data (a runtime-discovered type list) must see the same rows the following body writes, and each block's prefix reflects only that block's rows.

The block writer passes the column and its block slice.
Delegating composites (Nullable/Array/Tuple/Map/Nested) forward the outer column and row slice unchanged — byte-identical for every type supported today, since each inner's prefix is data-independent and ignores the arguments. Projecting an inner's own sliced sub-column (the flattened element sub-slice for element-space composites) is what a future data-dependent inner will need and lands later.
LowCardinality's prefix is a fixed version marker and ignores the slice.
Added a full-column WriteStatePrefix(codec, writer, column) convenience overload mirroring the existing WriteColumn helper.

J3 — `Variant(T1, ..., Tn)` BASIC discriminators

A discriminated union: each row holds a value of one alternative or NULL. Only the BASIC discriminators mode is supported (the server default over the native protocol); a COMPACT prefix is rejected.

Wire layout (per non-empty block): UInt64 mode word (0 = BASIC) → each alternative's own state prefix (empty for every supported alternative) → one UInt8 discriminator per row (255 = NULL) → a dense run per alternative, in declared order, holding the rows that selected it.

Alternatives are never Nullable — NULL is the discriminator's job — so, like Array/Tuple/Map/Nested, there is no Nullable(Variant(...)) round-trip; NULL composes inside instead.
The server sends the declared order canonicalized, so the codec does not reorder it.
Surfaced by a dense VariantColumn (discriminators + per-type child columns) — the zero-copy write source — with a precomputed per-row local index so measurement/addressing is O(1).
Ergonomic write source is a flat IColumn<object>, scattered by each value's runtime CLR type into per-type buffers via the tuple flat-write machinery.
Composes inside a composite (Array(Variant(...))) and takes a composite alternative (Variant(Array(T), ...)).
Its prefix is data-independent, so it needs neither the inner-projection part of J0a nor the prefix→data scratch machinery (deferred to Dynamic/J4).

Testing

807 tests pass, including 11 Variant unit tests (byte-anchored on the documented wire example) and 4 INSERT→SELECT round-trips against ClickHouse 26.6 (basic, three-alternative, composite alternative Variant(Array(UInt64), String), and Array(Variant(...))).
Coverage: VariantColumn 100%, VariantColumnCodec ~92% (remainder is defensive catch/guard paths).
Two stale tests that used Variant(...) as an "unsupported type" stand-in were repointed at Point (now that Variant resolves).
A review pass flagged and fixed an O(n²) dense-measurement path (now O(1) via the precomputed local index) and tightened CanWrite to reject a wrong-arity dense column.

Known limitation

On the ergonomic (IColumn<object>) write path, a variant whose alternatives share a convenience CLR write type (e.g. Variant(DateTime, DateTime64)) resolves ambiguously (lower discriminator wins). The dense VariantColumn path is unaffected. Noted for follow-up.

🤖 Generated with Claude Code

github-actions · 2026-07-22T14:26:28Z

Triage

Category: feature • Risk: high

Summary
This DRAFT PR adds Variant(T1, ..., Tn) support to the TCP native-protocol client. It consists of two bundled pieces: (J0a) a signature change to IColumnCodec.WriteStatePrefix — widened from (writer) to (writer, column, start, length) so data-dependent prefixes can observe their block's row slice — and (J3) the new VariantColumn, IVariantColumn, and VariantColumnCodec types implementing the BASIC discriminators wire layout. The codec supports both a zero-copy dense path (writing a VariantColumn read back from the server) and an ergonomic scatter path (a flat IColumn<object> bucketed by each value's runtime CLR type). All existing composite codecs (Nullable, Array, Tuple, Map, Nested, LowCardinality) were already updated in the base PR (#456) to forward the new WriteStatePrefix parameters.

What this impacts

ClickHouse.Driver.Tcp/Types/ — new VariantColumn.cs and Codecs/VariantColumnCodec.cs; ColumnCodecRegistry.cs now routes Variant to the new factory
Binary read path: multi-pass discriminator + per-type dense run reads with ArrayPool<byte> management
Binary write path (dense): O(1) slice addressing via precomputed LocalIndices; stackalloc int[children.Length] up to 255 ints per block write
Binary write path (ergonomic): reflection at constructor time (MakeGenericMethod / CreateDelegate) to build per-type bucket builders; boxing on the scatter path at write time
IColumnCodec.WriteStatePrefix interface — signature widened (base PR TCP J1/J2: Add LowCardinality(T) and LowCardinality(Nullable(T)) support #456 carries the cascading changes to all existing codecs)

Concerns

High rule — type system / binary protocol: New binary read and write paths for a discriminated union in Types/, including complex multi-pass wire encoding (mode word → per-type state prefixes → discriminator array → dense runs). Any off-by-one in LocalIndices precomputation or slice offset derivation would silently corrupt data.
High rule — reflection: MethodInfo.MakeGenericMethod + CreateDelegate is called in the VariantColumnCodec constructor for each alternative type. Construction is one-time (codec registry), not per-query, and the delegates are cached — but reflection is present.
Stacked PR / bundled concerns: The WriteStatePrefix interface widening (J0a) lives in the base TCP J1/J2: Add LowCardinality(T) and LowCardinality(Nullable(T)) support #456 PR; this PR's diff is clean on its own, but the two changes must be reviewed together. Confirm TCP J1/J2: Add LowCardinality(T) and LowCardinality(Nullable(T)) support #456 has merged or that reviewers are reading both diffs.
Draft status: PR is in DRAFT; not ready for merge.
Known ambiguity: The PR body documents that Variant(DateTime, DateTime64) — two alternatives sharing a convenience CLR write type — resolves non-deterministically (lower discriminator wins). Flagged as a known limitation, not a blocker, but worth explicit sign-off.

Required reviewer action

high — PR body must include an architectural description before review. (It already does; confirm it covers the WriteStatePrefix contract change and the J0a → J3 sequencing with respect to TCP J1/J2: Add LowCardinality(T) and LowCardinality(Nullable(T)) support #456 before marking ready.)

Copilot

Pull request overview

Adds TCP/native support for ClickHouse Variant(T1, ..., Tn) columns and updates the codec prefix-writing API so state prefixes can be written for the same row-slice as the subsequent column body (required for future data-dependent prefixes).

Changes:

Widened IColumnCodec.WriteStatePrefix to accept (column, start, length) and updated the block writer + composite codecs to forward the slice.
Implemented VariantColumn (dense discriminator + per-alternative child runs) and VariantColumnCodec (BASIC discriminators mode).
Added unit tests and INSERT→SELECT round-trip coverage for Variant, and repointed “unsupported type” tests from Variant(...) to Point.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ClickHouse.Driver.Tcp/Types/VariantColumn.cs	New dense column type for Variant with discriminator stream + child columns.
ClickHouse.Driver.Tcp/Types/IColumnCodec.cs	API change: `WriteStatePrefix` now receives `(column, start, length)`.
ClickHouse.Driver.Tcp/Types/ColumnCodecRegistry.cs	Registers `Variant(...)` factory in the default codec registry.
ClickHouse.Driver.Tcp/Types/ColumnCodecExtensions.cs	Adds full-column `WriteStatePrefix(codec, writer, column)` convenience overload.
ClickHouse.Driver.Tcp/Types/Codecs/VariantColumnCodec.cs	New Variant codec (BASIC mode) with dense + ergonomic write paths.
ClickHouse.Driver.Tcp/Types/Codecs/TupleColumnCodec.cs	Forwards slice parameters through `WriteStatePrefix`.
ClickHouse.Driver.Tcp/Types/Codecs/NullableColumnCodec.cs	Forwards slice parameters through `WriteStatePrefix`.
ClickHouse.Driver.Tcp/Types/Codecs/NestedColumnCodec.cs	Forwards slice parameters through `WriteStatePrefix`.
ClickHouse.Driver.Tcp/Types/Codecs/MapColumnCodec.cs	Forwards slice parameters through `WriteStatePrefix`.
ClickHouse.Driver.Tcp/Types/Codecs/LowCardinalityColumnCodec.cs	Updates prefix writer signature (prefix remains data-independent).
ClickHouse.Driver.Tcp/Types/Codecs/ArrayColumnCodec.cs	Forwards slice parameters through `WriteStatePrefix`.
ClickHouse.Driver.Tcp/Format/BlockWriter.cs	Writes state prefix with the same `(start, rowCount)` slice as the body.
ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs	Adds Variant INSERT→SELECT round-trip cases and server settings.
ClickHouse.Driver.Tcp.Tests/Types/VariantColumnCodecTests.cs	New byte-anchored unit tests for Variant codec.
ClickHouse.Driver.Tcp.Tests/Types/NullableColumnCodecTests.cs	Updates “unsupported inner” test to use `Point` instead of Variant.
ClickHouse.Driver.Tcp.Tests/Types/LowCardinalityColumnCodecTests.cs	Updates tests to use new `WriteStatePrefix` overload.
ClickHouse.Driver.Tcp.Tests/Types/ColumnCodecRegistryTests.cs	Updates “unsupported but well-formed” test to use `Point`.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

ClickHouse.Driver.Tcp/Types/VariantColumn.cs:69

VariantColumn precomputes LocalIndices by indexing counters[d] for every non-NULL discriminator, but it never validates that d is within the typeColumns range. If a malformed discriminator (or a VariantColumn constructed incorrectly) contains a value >= typeColumns.Length (and not 255), this will throw IndexOutOfRangeException during construction rather than a clear argument/format error.

        for (int row = 0; row < rowCount; row++)
        {
            byte d = discriminators[row];
            localIndex[row] = d == NullDiscriminator ? -1 : counters[d]++;
        }

+        // Map each alternative's writable CLR types to its discriminator, so the ergonomic write path can pick a
+        // row's alternative from the runtime type of its value. The canonical element type is registered first;
+        // if two alternatives claim the same CLR type the lower discriminator wins (the server does not allow
+        // duplicate alternative types, so this is only a defensive tie-break).
+        discriminatorByClrType = new Dictionary<Type, int>();
+        bool writable = true;
+        for (int i = 0; i < typeCount; i++)
+        {
+            childFlatBuilders[i] = (Func<string, string, object[], int, IColumn>)builderTemplate
+                .MakeGenericMethod(children[i].ElementType)
+                .CreateDelegate(typeof(Func<string, string, object[], int, IColumn>));
+
+            discriminatorByClrType.TryAdd(children[i].ElementType, i);
+            foreach (Type writeType in children[i].WritableElementTypes)
+            {
+                discriminatorByClrType.TryAdd(writeType, i);
+            }
+
+            // Probe writability with an empty child column so a Variant over a non-writable alternative (e.g.
+            // Nothing) is rejected up front rather than mid-write.
+            IColumn probe = childFlatBuilders[i](string.Empty, children[i].TypeName, Array.Empty<object>(), 0);
+            writable &= children[i].CanWrite(probe);
+        }


+            // The rebuilt wrapper borrows its children (freshly densified ones are unpooled, unchanged ones are
+            // still owned by the original), so it does not dispose them.
+            return new VariantColumn(column.Name, column.TypeName, denseSource.Discriminators.ToArray(), densified, column.RowCount, pooledDiscriminators: false, ownsColumns: false);


codecov · 2026-07-28T19:05:15Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Reads and writes the ClickHouse Variant composite: a fixed mode word, then the per-row discriminator byte stream, then each alternative's values in alternative order. The ergonomic insert source is a flat IColumn<object> whose values' runtime CLR types select their alternative, with null marking a NULL row. Also widens IColumnCodec.WriteStatePrefix from (writer) to (writer, column, start, length). A type whose prefix bytes derive from the data must see the same rows the following body writes, and each block's prefix reflects only that block's rows. Delegating composites forward the outer column and slice unchanged, which is byte-identical for every type supported today. Squashed from four commits. Three of them -- the prefix widening, the initial Variant support, and a densify step this range later dropped -- carried unresolved conflict markers across eight files, committed by a botched rebase resolution and then papered over by the fourth commit rewriting the same regions. Tip builds stayed green, so it went unnoticed. The tree here is that fourth commit's, so the markers and the non-compiling intermediate states are gone by construction. Co-Authored-By: Claude <noreply@anthropic.com>

…only PR 459 feedback: the discriminator map registered each alternative's convenience write types (e.g. DateTime for a DateTimeOffset alternative) as well as its canonical element type, but each per-alternative bucket is materialized as the canonical ElementType (BuildFlatColumn<ElementType>), so a convenience-typed value would fail the bucket cast with a deep InvalidCastException. Register only the canonical element type, so such a value is rejected up front with a clear "no alternative" error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The indexer read discriminators[row] straight out of the buffer, which is normally a pooled array longer than the column. That made an out-of-range row's behavior depend on whatever byte was left in the tail: a stale 255 reported the row as an existing NULL, while any other value fell through to the exactly-sized local-index array and threw. Indexing through the RowCount-sliced Discriminators span makes both spellings a bounds failure. The row count stays a parameter: each child column holds only the rows that selected it and a NULL row takes a slot in none of them, so the children say nothing about the height. The constructor validates the discriminator buffer instead, which also stops the local-index walk faulting on a short buffer with nothing naming the cause. Co-Authored-By: Claude <noreply@anthropic.com>

IVariantColumn already described the wire layout — discriminators, per-type child columns, and the precomputed per-row index into the selected child — so this promotes it rather than adding anything. Variant needs the columnar surface more than the other composites: it has no useful materialized element type, so its IColumn<T> surface is IColumn<object> and every row read through it is boxed. Dispatching on the discriminator and reading the selected child column is the only typed way in. NullDiscriminator moves from VariantColumn onto the interface. The interface doc refers to it, and VariantColumn is internal, so leaving it there would have left a public interface documented in terms of a constant no consumer can name.

Making IVariantColumn public in the previous commit turned the dense write path's trust boundary into a public extension point. The writer trusts invariants only VariantColumn's constructor establishes — every discriminator is a valid alternative index or the NULL marker, and LocalIndices is exactly as long as the column with a correct per-type running index — and it puts the discriminators on the wire before touching them, so a bad value from a caller-supplied implementation would desync the block mid-stream rather than fail cleanly. Gate on the concrete VariantColumn instead, which is what every other composite already does (ArrayValueColumn, MapColumn, NestedColumn, NullableValueColumn, LowCardinalityColumn) and what the LowCardinality split established one branch earlier. The interface stays public as a read view; anything else writable still arrives as IColumn<object> and takes the scattered path, which validates as it goes.

Say that a child column is the block's to dispose rather than the caller's, and that GetTypeColumn throws IndexOutOfRangeException outside [0, TypeCount) — which includes NullDiscriminator, the value the documented dispatch idiom reads straight off Discriminators, so it is the easy mistake to make.

Variant builds no per-slice write state, so it forwards its own column to every alternative's prefix phase and relies on each one ignoring it. For a Tuple alternative that lands in the tuple codec's forwarding walk over its children -- a live path with no test behind it: nothing in the suite paired Variant or Map with a Tuple, leaving that branch at zero coverage. Adds Variant(String, Tuple(UInt8, String)), and the same shape with a LowCardinality element inside the tuple so a child that carries its own state prefix has to reach the wire through the forwarded column. That second case is the one with teeth: dropping the forwarding walk desyncs the block mid-stream rather than throwing. Co-Authored-By: Claude <noreply@anthropic.com>

Variant was the only multi-child composite that built no write state. It wrote its prefix by handing its own column to every alternative and relying on each one ignoring it -- which held only because every alternative's prefix is data-independent, an invariant enforced from a different file (Dynamic is rejected inside Variant precisely because it would break it). It was also the reason Array, Tuple, Map and Nested each carry a branch for "an outer composite forwarded me a column I cannot project": Variant was the only forwarder that could hand a composite child a foreign column. BeginWrite now projects the slice into one column per alternative -- borrowed from the dense column, or scattered out of the boxed values -- and opens each alternative's own child state over it, which both phases then share. The per-alternative slicing is the same single pass WriteDense already did, so this moves work rather than adding it, and the body no longer re-derives it. Every alternative gets a column and a state even when no row selects it: the alternative set comes from the type, not the data, so each prefix belongs on the wire regardless. Two round-trip cases pin that -- a LowCardinality alternative and a Tuple-wrapping-LowCardinality alternative, each selected by no row, whose dictionary prefix must still be emitted. Two incidental improvements: the scatter now runs before any bytes are written, so a value with no matching alternative fails before the block is partly on the wire rather than after the prefix; and the wire output is unchanged, as the byte-exact codec tests confirm. Co-Authored-By: Claude <noreply@anthropic.com>

Map reaches its key and value codecs through a shape object rather than directly, so it is the composite whose own write state is most easily bypassed, and nothing paired it with a variant. The value is a LowCardinality so the case also pins that a dictionary prefix two composites down still reaches the wire. Also drops two comments on the Tuple-alternative cases that described the forwarding design the previous commit removed. Co-Authored-By: Claude <noreply@anthropic.com>

Same split as the other composites. Co-Authored-By: Claude <noreply@anthropic.com>

github-actions Bot added triage:feature risk:high labels Jul 22, 2026

alex-clickhouse requested a review from Copilot July 22, 2026 14:26

Copilot started reviewing on behalf of alex-clickhouse July 22, 2026 14:27 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread ClickHouse.Driver.Tcp/Types/VariantColumn.cs

Comment thread ClickHouse.Driver.Tcp/Types/Codecs/VariantColumnCodec.cs Outdated

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 8ce4435 to 436da05 Compare July 22, 2026 15:26

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 436da05 to 32cbdaa Compare July 22, 2026 16:22

alex-clickhouse force-pushed the tcp/epic-j3-variant branch 2 times, most recently from bd760dc to d9ad7e0 Compare July 23, 2026 06:53

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from d9ad7e0 to b34f599 Compare July 23, 2026 08:54

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from b34f599 to 63e0070 Compare July 23, 2026 09:01

alex-clickhouse mentioned this pull request Jul 23, 2026

TCP J4: Add Dynamic support #460

Draft

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 63e0070 to 1c14219 Compare July 23, 2026 11:47

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 1c14219 to 1feb65c Compare July 23, 2026 18:04

alex-clickhouse requested a review from Copilot July 24, 2026 07:11

Copilot started reviewing on behalf of alex-clickhouse July 24, 2026 07:11 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 1feb65c to 38ee038 Compare July 24, 2026 07:19

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 38ee038 to f57f502 Compare July 24, 2026 07:26

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from f57f502 to 8d9bbc3 Compare July 24, 2026 08:39

alex-clickhouse force-pushed the tcp/epic-j3-variant branch 2 times, most recently from eff12ec to 566a0be Compare July 28, 2026 18:59

alex-clickhouse force-pushed the tcp/epic-j3-variant branch 2 times, most recently from d131fbd to 93937df Compare July 29, 2026 07:05

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 93937df to d09236a Compare July 29, 2026 13:26

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from d09236a to be8298c Compare July 29, 2026 15:47

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from f149f19 to a05d662 Compare July 30, 2026 08:37

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from a05d662 to c576a45 Compare July 30, 2026 09:10

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from c576a45 to 6074afb Compare July 30, 2026 09:47

alex-clickhouse and others added 10 commits July 31, 2026 18:12

Reject a foreign write state in Variant

d4536a2

Same split as the other composites. Co-Authored-By: Claude <noreply@anthropic.com>

alex-clickhouse force-pushed the tcp/epic-j3-variant branch from 6074afb to d4536a2 Compare July 31, 2026 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

TCP J3: Add Variant(...) support - #459

TCP J3: Add Variant(...) support#459
alex-clickhouse wants to merge 10 commits into
tcp/epic-j1-lowcardinalityfrom
tcp/epic-j3-variant

alex-clickhouse commented Jul 22, 2026

Uh oh!

github-actions Bot commented Jul 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

alex-clickhouse commented Jul 22, 2026

J0a — WriteStatePrefix receives the sliced column

J3 — Variant(T1, ..., Tn) BASIC discriminators

Testing

Known limitation

Uh oh!

github-actions Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Triage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

codecov Bot commented Jul 28, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

J0a — `WriteStatePrefix` receives the sliced column

J3 — `Variant(T1, ..., Tn)` BASIC discriminators

github-actions Bot commented Jul 22, 2026 •

edited

Loading