Skip to content

TCP J4: Add Dynamic support - #460

Draft
alex-clickhouse wants to merge 11 commits into
tcp/epic-j3-variantfrom
tcp/epic-j4-dynamic
Draft

TCP J4: Add Dynamic support#460
alex-clickhouse wants to merge 11 commits into
tcp/epic-j3-variantfrom
tcp/epic-j4-dynamic

Conversation

@alex-clickhouse

Copy link
Copy Markdown
Collaborator

Adds the ClickHouse Dynamic column type over the native protocol, stacked on #459 (Variant).

Dynamic is a column whose per-row value type is discovered at runtime. This reads and writes only the FLATTENED serialization (wire version 3), gated on the query setting output_format_native_use_flattened_dynamic_and_json_serialization = 1; the leading UInt64 version word is validated and any non-flat encoding rejected as a protocol error. (A follow-up will have the client set that setting automatically — tracked in the TODO.)

What's here

  • Read + write of top-level Dynamic (FLATTENED v3): version + runtime type-name list + per-type prefixes as the state prefix, then discriminators (width scales with type count; NULL = num_types) + one dense run per type. Verified against a real 26.6 server; the documented-bytes unit tests use a real server capture.
  • Dense DynamicColumn (type-name list + widened discriminators + per-type child columns) — the zero-copy read/write source; an ergonomic IColumn<object> write source infers each value's ClickHouse type via a self-contained DynamicTypeInference (the TCP project can't reference the main driver): scalars, IPv4/IPv6 by address family, date-times, decimals, and Array/Map/Tuple recursion, normalizing each value to its codec's element type.
  • The J0b write-state contract: IColumnCodec.BeginWrite + IColumnWriteState, with state-aware WriteStatePrefix/WriteColumn overloads that default to the existing state-free ones (leaf codecs untouched). The block writer runs BeginWrite → prefix → body → Dispose, so a data-dependent prefix (the Dynamic type list) and the element-flattening composites do their flatten/scatter once across both phases.
  • Full nesting, both directions: Array/Tuple/Map/Nested now project their flattened inner sub-column into the child's prefix and body, so Array(Dynamic), Tuple(...Dynamic...), Map(K, Dynamic), Nested(...Dynamic...) and composite values inside a Dynamic all round-trip. Variant rejects a Dynamic alternative (server-disallowed).
  • Like Variant/Array/Tuple/Map/Nested, there is no Nullable(Dynamic) — NULL rides the discriminator.

Testing

  • Codec-level documented-bytes unit tests (write→bytes, bytes→read, dense round-trip, version-not-3 rejection, discriminator-past-count, width boundaries) and inference-mapping unit tests.
  • Integration round-trip cases (CREATE/INSERT/SELECT against a live server): top-level scalars + NULL, a "(basically) every type + composites" mega-case, DateTimeOffset/DateTime/decimal inference, and Array(Dynamic), Tuple(Dynamic, String), Tuple(Dynamic, Dynamic), Map(String, Dynamic), Nested(a Dynamic, b String), Array(Tuple(Dynamic, String)).
  • Full suite green (869 tests); DynamicColumnCodec ~90% line coverage (remaining gaps are the >255-type discriminator-width branches).

Notes / deferred

  • Composite-of-coercion-needing-element (e.g. DateTimeOffset[] inside a Dynamic) throws a clear NotSupportedException; use the canonical CLR type (ClickHouseDateTime64/ClickHouseDecimal).

🤖 Generated with Claude Code

@alex-clickhouse
alex-clickhouse requested a review from Copilot July 23, 2026 10:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Triage

Category: featureRisk: high

Summary
This PR adds read/write support for the ClickHouse Dynamic column type over the native TCP protocol, stacked on #459 (Variant). It introduces a new DynamicColumnCodec implementing only the "flattened" serialization (version 3, gated on the server query setting output_format_native_use_flattened_dynamic_and_json_serialization=1), a DynamicColumn dense-column shape, and a self-contained DynamicTypeInference helper that derives ClickHouse type names from CLR values at write time via recursive inference. The PR also threads a BeginWrite/IColumnWriteState write-state contract through composite codecs (Array, Tuple, Map, Nested) so the data-dependent type list is computed once and shared across the prefix and body write phases — enabling full nesting of Dynamic in both read and write directions. Coverage includes codec unit tests against captured server bytes, inference mapping tests, and integration round-trip cases for scalars, NULLs, every supported type in one column, composite nesting, datetime/decimal inference, and overflow buckets.

What this impacts

  • Binary protocol (TCP) — new serialization layout for Dynamic: version word, varuint type count, type-name list, per-type state prefixes, variable-width discriminators (1/2/4 bytes scaled by type count), and one dense run per type
  • ClickHouse.Driver.Tcp/Types/Codecs/ — new DynamicColumnCodec (613 lines), DynamicTypeInference (217 lines), DynamicWire (32 lines)
  • ClickHouse.Driver.Tcp/Types/ — new DynamicColumn and IDynamicColumn interface (210 lines)
  • ClickHouse.Driver.Tcp/Types/ColumnCodecRegistry.cs — Dynamic factory registration
  • ClickHouse.Driver.Tcp.Tests/ — 308 lines of new codec and inference unit tests, plus 178 lines of new InsertRoundTripCase entries covering full nesting

Concerns

  • Reflection in write pathDynamicColumnCodec.FlatBuilderFor uses MethodInfo.MakeGenericMethod/CreateDelegate via ConcurrentDictionary to build per-element-type flat-column builders. The result is cached (one-time cost per CLR element type), but reflection fires the high-risk performance rule; a reviewer should confirm the cache is sufficient and contention on ConcurrentDictionary is negligible.
  • Unbounded recursion in type inferenceDynamicTypeInference.Infer recurses into composite elements (Array → element, Map → key/value, Tuple → fields) with no depth guard, and composites may themselves contain composites. An adversarially or accidentally deeply nested input (e.g. Array(Array(Array(...)))) could overflow the stack; the high-risk recursion-on-unbounded-input rule fires.
  • DRAFT + stacked on TCP J3: Add Variant(...) support #459 — this diff is not standalone; the IColumnWriteState/BeginWrite interface and the updated composite codecs live in TCP J3: Add Variant(...) support #459. The full risk surface cannot be assessed without that stack, and this PR should not be merged before TCP J3: Add Variant(...) support #459.
  • Server opt-in required — only flattened serialization (version 3) is implemented; a TODO defers automatic injection of output_format_native_use_flattened_dynamic_and_json_serialization=1, meaning callers must set it manually today.

Required reviewer action

  • PR body must include an architectural description before review (high-risk policy). Additionally, TCP J3: Add Variant(...) support #459 must land and be reviewed first; review this PR's diff against that branch, not main.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (2)

ClickHouse.Driver.Tcp/Types/Codecs/TupleColumnCodec.cs:367

  • In the flat tuple write path, BuildState returns pooled childBoxed buffers in finally, but if one of the child codecs’ BeginWrite(...) calls throws, any earlier child states are leaked (they may hold rented buffers) because they’re never disposed on the exceptional path.
        try
        {
            for (int row = 0; row < length; row++)
            {
                var tuple = (ITuple)column.GetValue(start + row);
                for (int i = 0; i < arity; i++)
                {
                    childBoxed[i][row] = tuple[i];
                }
            }

            for (int i = 0; i < arity; i++)
            {
                childColumns[i] = childFlatBuilders[i](column.Name, children[i].TypeName, childBoxed[i], length);
                childStates[i] = children[i].BeginWrite(childColumns[i], 0, length);
            }

            return new TupleWriteState { ChildColumns = childColumns, ChildStart = 0, Length = length, ChildStates = childStates };

ClickHouse.Driver.Tcp/Types/Codecs/MapColumnCodec.cs:392

  • In the jagged map path, if one of the inner BeginWrite(...) calls throws (especially valueCodec.BeginWrite after keyState is created), the already-created inner state is leaked. The exception path should dispose any created inner state before returning the pooled key/value buffers.
            var keyColumn = ArrayColumn<TKey>.OverBuffer(column.Name, keyCodec.TypeName, flatKeys, total);
            var valueColumn = ArrayColumn<TValue>.OverBuffer(column.Name, valueCodec.TypeName, flatValues, total);
            IColumnWriteState keyState = keyCodec.BeginWrite(keyColumn, 0, total);
            IColumnWriteState valueState = valueCodec.BeginWrite(valueColumn, 0, total);
            return new MapWriteState(keyColumn, valueColumn, pairBase: 0, total, keyState, valueState, flatKeys, flatValues);
        }
        catch
        {
            ArrayPool<TKey>.Shared.Return(flatKeys, clearArray: RuntimeHelpers.IsReferenceOrContainsReferences<TKey>());
            ArrayPool<TValue>.Shared.Return(flatValues, clearArray: RuntimeHelpers.IsReferenceOrContainsReferences<TValue>());
            throw;

Comment on lines +31 to +38
public static void WriteFull(this IColumnCodec codec, ClickHouseBinaryWriter writer, IColumn column)
{
IColumnWriteState state = codec.BeginWrite(column, 0, column.RowCount);
try
{
codec.WriteStatePrefix(writer, column, 0, column.RowCount, state);
codec.WriteColumn(writer, column, 0, column.RowCount, state);
}
Comment on lines +333 to +341
if (column is ITupleColumn dense && dense.Children.Count == arity)
{
for (int i = 0; i < arity; i++)
{
childColumns[i] = dense.Children[i];
childStates[i] = children[i].BeginWrite(dense.Children[i], start, length);
}

return new TupleWriteState { ChildColumns = childColumns, ChildStart = start, Length = length, ChildStates = childStates };
Comment on lines +332 to +340
var fieldColumns = new IColumn[children.Length];
var fieldStates = new IColumnWriteState[children.Length];
for (int f = 0; f < children.Length; f++)
{
children[f].WriteColumn(writer, nested.GetField(f), elementBase, elementCount);
fieldColumns[f] = nested.GetField(f);
fieldStates[f] = children[f].BeginWrite(fieldColumns[f], elementBase, elementCount);
}

return new NestedWriteState { FieldColumns = fieldColumns, ElementBase = elementBase, ElementCount = elementCount, FieldStates = fieldStates };
Comment on lines +328 to +335
if (column is MapColumn<TKey, TValue> dense)
{
// Dense form (the wire's own layout): the offsets already exist and the key/value columns already hold
// every pair, so write them directly. The wire offsets are relative to this slice's pair streams, so
// subtract the slice's first pair index from each.
ReadOnlySpan<int> offsets = dense.Offsets;
int pairBase = offsets[start];
for (int i = 0; i < length; i++)
{
writer.WriteUInt64((ulong)(offsets[start + i + 1] - pairBase));
}

int pairCount = offsets[start + length] - pairBase;
keyCodec.WriteColumn(writer, dense.KeyColumn, pairBase, pairCount);
valueCodec.WriteColumn(writer, dense.ValueColumn, pairBase, pairCount);
return;
IColumnWriteState keyState = keyCodec.BeginWrite(dense.KeyColumn, pairBase, pairCount);
IColumnWriteState valueState = valueCodec.BeginWrite(dense.ValueColumn, pairBase, pairCount);
return new MapWriteState((IColumn<TKey>)dense.KeyColumn, (IColumn<TValue>)dense.ValueColumn, pairBase, pairCount, keyState, valueState, keyBuffer: null, valueBuffer: null);
@alex-clickhouse

Copy link
Copy Markdown
Collaborator Author

Thanks @copilot — all four inline findings plus the two low-confidence ones were valid (the same pooled-buffer / partial-child-state leak class). Addressed in the latest push:

  • ColumnCodecExtensions.WriteFull — now returns early for a zero-row column, matching BlockWriter (which skips prefix+body for empty slices), so it can't emit bytes the block writer never would or trip a codec that rejects empty ranges.
  • TupleColumnCodec.BuildState (dense + flat paths) — both loops now dispose the child write states already built (via a DisposeStates helper) if a later child's BeginWrite throws.
  • NestedColumnCodec.BuildState — the field-state loop now disposes already-created field states on a mid-loop throw.
  • MapColumnCodec MapShape.BeginWrite (dense + jagged paths) — a keyState created before valueCodec.BeginWrite throws is now disposed before rethrowing (jagged path also still returns the pooled key/value buffers).

These mirror the try/catch cleanup already applied to DynamicColumnCodec's BuildScatteredState/BuildDenseState. Full suite green (869 tests).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment on lines +120 to +131
// Prefer a present element's runtime type (so IPAddress and other value-disambiguated types resolve), and
// fall back to the declared element type for an empty (or all-null) array.
foreach (object element in array)
{
if (element is not null)
{
return $"Array({InferComposable(element)})";
}
}

return $"Array({InferFromClrType(elementType)})";
}
Comment on lines +173 to +182
private static string InferFromClrType(Type type)
{
if (type is not null && Scalars.TryGetValue(type, out string scalar))
{
return scalar;
}

throw new NotSupportedException(
$"No ClickHouse type is inferred for a Dynamic array whose element CLR type is '{type?.ToString() ?? "unknown"}' with no element to disambiguate it.");
}
Comment on lines +506 to +512
private static Func<string, string, object[], int, IColumn> FlatBuilderFor(Type elementType)
=> FlatBuilders.GetOrAdd(elementType, static type => (Func<string, string, object[], int, IColumn>)
typeof(DynamicColumnCodec)
.GetMethod(nameof(BuildFlatColumn), BindingFlags.NonPublic | BindingFlags.Static)
.MakeGenericMethod(type)
.CreateDelegate(typeof(Func<string, string, object[], int, IColumn>)));

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-j4-dynamic branch 2 times, most recently from 8d169a3 to da490e0 Compare July 29, 2026 07:05
@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-j4-dynamic branch 3 times, most recently from 6c84eda to de76a44 Compare July 30, 2026 08:13
alex-clickhouse and others added 10 commits July 31, 2026 18:12
Adds the ClickHouse `Dynamic` column type — a column whose per-row value
type is discovered at runtime — over the native protocol, reading and
writing only the FLATTENED serialization (wire version 3), gated on
`output_format_native_use_flattened_dynamic_and_json_serialization = 1`.
The leading version word is validated and any non-flat encoding rejected.

- Dense `DynamicColumn` (runtime type-name list + widened discriminators +
  per-type child columns) as the zero-copy read/write source; an ergonomic
  `IColumn<object>` write source infers each value's ClickHouse type via a
  self-contained `DynamicTypeInference` (scalars, IPv4/IPv6 by address
  family, date-times, decimals, and Array/Map/Tuple recursion), normalizing
  a value to its codec's element type so it round-trips.
- Introduces the per-operation write-state contract (`IColumnCodec.BeginWrite`
  + `IColumnWriteState` + state-aware `WriteStatePrefix`/`WriteColumn`
  overloads that default to the state-free ones, so leaf codecs are
  untouched). The block writer runs BeginWrite -> prefix -> body -> Dispose.
  This lets a data-dependent prefix (the Dynamic type list) and the
  element-flattening composites do their flatten/scatter once across both
  phases.
- Full nesting both directions: the Array/Tuple/Map/Nested codecs now project
  their flattened inner sub-column into the child's prefix and body, so
  `Array(Dynamic)`, `Tuple(...Dynamic...)`, `Map(K, Dynamic)`,
  `Nested(...Dynamic...)` and composite values inside a Dynamic all
  round-trip. Variant rejects a Dynamic alternative (server-disallowed).
- Like Variant, there is no `Nullable(Dynamic)` — NULL rides the
  discriminator (value `num_types`, not 255).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dynamic infers its alternatives from the data at write time, so it cannot densify
up front (its Densify stays the identity) — but the per-type columns it builds
from the boxed values are the ergonomic form, and the resolved codecs now write
and measure only the dense wire shape. So both data-dependent paths densify each
per-type column before delegating: BuildState densifies each bucket before the
child's BeginWrite (storing the dense column for the body phase), and the
per-row measure densifies its one-row probe before pricing it. This makes a
Dynamic value that is itself an Array/Tuple/Nullable round-trip end-to-end.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dynamic discovers its per-type children from the ergonomic source and
writes each straight through its codec, with no densify pre-pass or byte
measurement.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR 460 feedback:
- Reject a heterogeneous Dynamic array (e.g. IPv4 mixed with IPv6 in an
  IPAddress[]) with a clear NotSupportedException at inference time,
  rather than letting a later element fail the bucket cast mid-write.
- Infer an empty nested composite (an empty Array(Array(T)) or
  Array(Map(...))) structurally from its CLR element type instead of
  throwing, so empty nested arrays are usable inside Dynamic.
- Null-check the BuildFlatColumn reflection lookup in FlatBuilderFor,
  throwing an actionable InvalidOperationException like VariantColumnCodec
  rather than a NullReferenceException.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ReadColumn_DocumentedBytes_ReconstructsValuesAndNull asserted only
RowCount + GetValue on the golden vector, which the "Dynamic [scalars + null]"
case in InsertRoundTripCase reproduces against a real server with the same
{String, UInt64} type list and NULL discriminator.

Rather than delete it, folded it into
ReadColumn_DocumentedBytes_SurfacesTheRuntimeTypeList -- same setup, and
keeping the value assertions next to the type list and discriminators guards
the decoder against drifting even if a future server emits a different type
ordering. Net one test, no coverage lost.

Added the Dynamic(max_types=2) case. That is the one shape where the server
reshapes the dynamic structure -- types past N go to the shared/overflow
bucket -- and Create_MaxTypesArgument_IsAccepted only checks the TypeName
string, so the client's read of the reshaped structure had no coverage at all.

Verified against ClickHouse 26.6 -- 1009 tests pass, integration included.

Co-Authored-By: Claude <noreply@anthropic.com>
Same defect as Variant, which Dynamic's indexer mirrors: it read discriminators[row] out of a
buffer that is normally a pooled array longer than the column, so an out-of-range row's
behavior depended on the leftover value — a stale NULL discriminator reported the row as an
existing NULL, anything else fell through to the exactly-sized local index and threw. Indexing
through the RowCount-sliced Discriminators span makes both a bounds failure, and the
constructor now validates the buffer rather than letting the local-index walk fault on a short
one.

Co-Authored-By: Claude <noreply@anthropic.com>
IDynamicColumn already described the wire layout, so this promotes it and
documents the two things that distinguish Dynamic from Variant for a consumer
reading it columnar: the runtime type list is discovered per block rather than
declared, so TypeNames is how a caller knows which typed column to read a child
as; and because the list is discovered, NULL is encoded as TypeCount — one past
the last type — instead of Variant's fixed 255.

Like Variant, Dynamic has no useful materialized element type, so its
IColumn<T> surface is IColumn<object> and the columnar view is the typed way in.
Same regression as the variant branch: making IDynamicColumn public turned the
dense write path's trust boundary into a public extension point. The planner
trusts invariants only DynamicColumn's constructor establishes — the type-name
list matches the child-column count, and every discriminator is a valid type
index or the NULL marker — so gate on the concrete class, as every other
composite does. The interface stays public as a read view.

Two consequences of the same exposure, fixed alongside:

TypeNames returned the backing string[] as IReadOnlyList<string>, which is
castable back and mutable. A caller could rewrite a decoded column's type list,
after which a re-insert resolves a codec from the rewritten name while the child
column still holds the original values. Hand out a read-only wrapper instead.

BuildDenseState rented its discriminator slice before the per-type walk that
indexes by discriminator, with nothing between the rent and the try that would
return it. Reordered so the rent comes after the fallible indexing. Unreachable
again now the gate is concrete, but it cost nothing to stop relying on that.
Same two additions as the variant branch, plus a note that TypeNames is read-only
and does not hand out its backing storage.
Last of the six composites.

Co-Authored-By: Claude <noreply@anthropic.com>
A Map column now takes dictionary rows as well as pair arrays, but a
dictionary handed to a Dynamic column was still rejected outright: no
ClickHouse type was inferred for it. Dynamic derives each value's type from
its runtime shape, and only a KeyValuePair<K, V>[] was recognized as a map.

Any IReadOnlyDictionary or IDictionary now infers as that same Map(K, V) and
is coerced to the pair array the map codec reads back, the way inference
already coerces a DateTimeOffset to a raw nanosecond count and a decimal to
ClickHouseDecimal. The coercion is what makes it correct rather than cosmetic:
Dynamic materializes each per-type bucket as the child codec's element type,
so an uncoerced dictionary would fail that cast mid-write. Coercing also means
the two spellings of one map type share a single wire type and a single run,
instead of colliding as two — a Dynamic column's type list is derived from the
data, so that collapse has to happen here or it is persisted.

Because the type is derived from the CLR key and value types alone (as the
pair-array spelling already was), a key or value that needs a value to
disambiguate it — IPAddress, DateTimeOffset, decimal, a nested dictionary —
is rejected before any byte is written, now naming the dictionary and the side
at fault rather than reporting a bare CLR type the caller never wrote.

Two hazards the interface walk brings, both rejected rather than guessed:

- A type that is a dictionary of two different key/value pairs. Interface
  order is unspecified, so picking the first would let a runtime upgrade that
  reorders them start inferring the other Map type for the same value.
- A multidimensional array, which enumerates as its elements and so used to
  infer Array(element) and fail the bucket cast mid-write. Pre-existing for
  the pair-array and plain-array spellings; a dictionary value type reaches it
  too, so it is now a clear rejection for all three.

The plan (type string plus coercion) is cached per dictionary CLR type, so the
interface walk and the closed generic happen once, not per row; a non-
dictionary type is deliberately not cached, since the only caller throws right
after and a negative entry would pin a caller's Type — and any collectible
load context behind it — forever. The pair array is sized from the
dictionary's own Count, with the enumeration taken as truth if the two
disagree, so a row mutated mid-insert cannot leave default pairs in it.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants