Skip to content

TCP K: geo aliases, Geometry, SimpleAggregateFunction and AggregateFunction - #577

Draft
alex-clickhouse wants to merge 1 commit into
tcp/epic-b3-compressionfrom
tcp/epic-k-type-aliases
Draft

TCP K: geo aliases, Geometry, SimpleAggregateFunction and AggregateFunction#577
alex-clickhouse wants to merge 1 commit into
tcp/epic-b3-compressionfrom
tcp/epic-k-type-aliases

Conversation

@alex-clickhouse

@alex-clickhouse alex-clickhouse commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Four families of type name the server puts in a column header that the client had no factory for. Each one failed the whole query with ClickHouse type '...' is not supported by this client yet.

Geo aliases

Point is Tuple(Float64, Float64); Ring and LineString are Array(Point); Polygon and MultiLineString are arrays over those; MultiPolygon is Array(Polygon). None needs a codec — only a registration that resolves the structure. They surface the structural CLR value ((double, double) up to (double, double)[][][]), which is what the TCP parameter formatter already accepted for these names and structurally what the HTTP driver surfaces.

The name on the wire is always the alias — a header says Point, never Tuple(Float64, Float64) — so ArrayColumnCodec.Create, TupleColumnCodec.Create and VariantColumnCodec.Create take an optional type name and report the alias. The aliases are defined in terms of each other, mirroring the HTTP driver's class hierarchy, so a Polygon's inner codec calls itself Ring.

Ring/LineString and Polygon/MultiLineString are structurally identical, so each has its own round-trip case: a registration pointing one at the other's codec would still pass the other's case.

Geometry, and a Variant fix that is not Geometry-specific

Geometry is an alias to Variant(LineString, MultiLineString, MultiPolygon, Point, Polygon, Ring) — the server's canonical name-sorted order, which nothing on the wire carries.

Four of the six alternatives collide in pairs on their CLR type, so a value of a shared shape names no alternative. VariantColumnCodec previously resolved this with a Dictionary.TryAdd tie-break, silently writing a Ring as a LineString. Colliding types are now struck from the map and refused by name.

The refusal is per value, not per column. An IColumn<object> says nothing about the runtime types it holds, so refusing the whole column would also reject every unambiguous value in it — and Variant(IPv4, IPv6, String) is legal with default settings, where the two IP alternatives collide but a string does not. It must keep working, and it does. Same for ulong in Variant(JSON, String, UInt64). This also settles the Variant(JSON, String) ambiguity left open by J5.

Aggregate states

SimpleAggregateFunction(func, T) encodes as a bare T, so it resolves to T's codec itself rather than a renaming wrapper, which would cost a virtual call per operation for a cosmetic type name. The inner goes back through the registry, so a composite, nullable or itself-aliased T all work.

AggregateFunction is refused with a merge query that actually runs. The combinator attaches to the bare function name and a parameterized function keeps its parameters ahead of the column, so AggregateFunction(quantiles(0.5, 0.9), UInt64) suggests quantilesMerge(0.5, 0.9)(column) — not quantiles(0.5, 0.9)Merge(column), which is not a function, and not a bare quantilesMerge(column), which the server rejects for wanting its parameters.

Testing

Round-trip corpus cases for every alias and for three SimpleAggregateFunction shapes; codec unit tests for the resolution surface and the refusal paths only, per the test-layering rules.

GeometryIntegrationTests pins the discriminator order against the server. The corpus cannot: transposing a structurally identical pair emits a byte-identical block. Nor is it enough to write discriminator i and ask the server what it calls i — that answers from the server's own list. The expectation is read out of the codec and compared with the server's name for the row written at each position; mutation-checked by transposing Ring and LineString, which the first version of the test did not catch.

Coverage on the new files is 100%; the touched composites are 91–96%.

Geometry is gated on TcpFeature.Geometry — on the 25.8 CI floor the server resolves it to String at DDL time, so an ungated case would silently exercise a String column there. Verified by enumerating the corpus at CLICKHOUSE_VERSION=25.8: the Geometry cases disappear, the geo aliases stay.

🤖 Generated with Claude Code

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

Adds TCP/native support for geo aliases, Geometry, and aggregate-function column types while improving ambiguous Variant handling.

Changes:

  • Registers structural codecs for geo and aggregate aliases.
  • Rejects ambiguous Variant CLR mappings.
  • Adds unit and integration coverage.

Reviewed changes

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

Show a summary per file
File Description
ColumnCodecRegistry.cs Registers new type factories.
VariantColumnCodec.cs Handles ambiguous CLR alternatives and aliases.
TupleColumnCodec.cs Supports alias type names.
GeoColumnCodecs.cs Defines geo alias structures.
ArrayColumnCodec.cs Supports alias type names.
AggregateFunctionColumnCodecs.cs Resolves or rejects aggregate types.
InsertRoundTripCase.cs Adds round-trip cases.
VariantColumnCodecTests.cs Tests Variant ambiguity behavior.
NullableColumnCodecTests.cs Updates unsupported-type coverage.
ColumnCodecRegistryTests.cs Tests new registrations and diagnostics.
PocoWriteIntegrationTests.cs Tests Geometry POCO refusal.
GeometryIntegrationTests.cs Validates Geometry discriminator ordering.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

// The geo aliases name structures the codecs above already encode — Point is a two-element Float64 tuple and
// the rest are arrays over it. The server puts the alias in the column header, so each resolves its structure
// and keeps its own name. Ring and LineString share a structure, as do Polygon and MultiLineString.
AddFactory("Point", static (TypeNode _, in ResolveContext context, ColumnCodecRegistry registry) => GeoColumnCodecs.CreatePoint(in context, registry));
Comment on lines +106 to +110
if (!discriminatorByClrType.TryAdd(children[i].ElementType, i))
{
ambiguousClrTypes.Add(children[i].ElementType);
discriminatorByClrType.Remove(children[i].ElementType);
}
@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-k-type-aliases branch from ce7b178 to 0ca474d Compare August 21, 2026 13:07
@codecov

codecov Bot commented Aug 21, 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-k-type-aliases branch 2 times, most recently from 5919ae3 to 68b1c8a Compare August 22, 2026 17:06
Epic K, minus QBit. Four type names the server puts in a column header
that the client had no factory for, so each one failed the whole query
with "not supported by this client yet".

Point is Tuple(Float64, Float64) and Ring, LineString, Polygon,
MultiLineString and MultiPolygon are arrays layered over it, so none
needs a codec: only a registration that resolves the structure. The
name on the wire is always the alias, though, so ArrayColumnCodec,
TupleColumnCodec and VariantColumnCodec take an optional type name and
report the alias rather than the structure it stands for. The aliases
are defined in terms of each other, so a Polygon's inner codec calls
itself Ring.

Geometry is an alias to a Variant over those six. Its order is the
server's name-sorted one, which nothing on the wire carries, so
GeometryIntegrationTests compares the client's list against what the
server calls each discriminator. Four of the six alternatives are
structurally identical in pairs, so a value of a shared shape names no
alternative: those are struck from the write map and refused by name,
while the unambiguous ones still write. That is per value rather than
per column on purpose -- an IColumn<object> says nothing about the
runtime types it holds, and Variant(IPv4, IPv6, String) must keep
taking a string even though its two IP alternatives collide. It also
settles the Variant(JSON, String) ambiguity left open by J5.

SimpleAggregateFunction(func, T) encodes as a bare T, so it resolves to
T's codec itself -- no renaming wrapper, which would cost a virtual
call per operation for a cosmetic name. AggregateFunction is refused
with a merge query that runs: the combinator attaches to the bare
function name, and a parameterized one keeps its parameters ahead of
the column, so quantiles suggests quantilesMerge(0.5, 0.9)(column).

Geometry is gated on TcpFeature.Geometry; on the 25.8 floor the server
resolves it to String at DDL time. The geo aliases and the aggregate
types need no gate.

Co-Authored-By: Claude <noreply@anthropic.com>
@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-k-type-aliases branch from 68b1c8a to 922a9fa Compare August 22, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants