Box-free POCO read fast path (#509) - #449
Open
alex-clickhouse wants to merge 9 commits into
Open
Conversation
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new box-free materialization fast path for ClickHouseClient.QueryAsync<T>(), reducing per-cell allocations by reading values directly from the response stream into POCO properties. It extends the type system with typed read support (ITypedReader<T>) and adds selective per-column fallback so composite columns don’t disable fast reads for scalar siblings.
Changes:
- Add
ITypedReader<T>and implement typed, non-boxing reads across scalar/value ClickHouse types (and selected multi-representation reads likeDateTimeOffset,DateOnly,byte[], nativeInt128/UInt128on .NET 8+). - Implement a POCO read fast-path pipeline: expression factory dispatch (
PocoReadExpressionFactory), assignment rule sharing (PocoColumnAssignment), and cached per-shape row materializers (PocoTypeRegistry). - Update
QueryAsync<T>()to use the new fast path when available, and add a dedicated test suite validating parity and binding behavior.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| RELEASENOTES.md | Document allocation/perf impact and new QueryAsync<T> binding capabilities. |
| CHANGELOG.md | Changelog entry for the new POCO read fast path and multi-representation bindings. |
| ClickHouse.Driver/Types/ITypedReader.cs | Introduce ITypedReader<T> to enable non-boxing deserialization. |
| ClickHouse.Driver/Types/AbstractDateTimeType.cs | Add typed reads for DateTime/DateTimeOffset/DateOnly and centralize canonical boxed read. |
| ClickHouse.Driver/Types/DateType.cs | Adapt Date decoding to the new AbstractDateTimeType read model. |
| ClickHouse.Driver/Types/Date32Type.cs | Adapt Date32 decoding to the new AbstractDateTimeType read model. |
| ClickHouse.Driver/Types/DateTimeType.cs | Split instant decoding and add typed DateTimeOffset support via base hooks. |
| ClickHouse.Driver/Types/DateTime64Type.cs | Share wire read between DateTime and DateTimeOffset typed reads. |
| ClickHouse.Driver/Types/AbstractBigIntegerType.cs | Add typed BigInteger read path to support fast materialization. |
| ClickHouse.Driver/Types/Int128Type.cs | Add native Int128 typed read on .NET 8+. |
| ClickHouse.Driver/Types/UInt128Type.cs | Add native UInt128 typed read on .NET 8+. |
| ClickHouse.Driver/Types/DecimalType.cs | Add typed reads for decimal and ClickHouseDecimal, keeping boxed behavior canonical. |
| ClickHouse.Driver/Types/StringType.cs | Provide both string and byte[] typed representations while preserving boxed mode selection. |
| ClickHouse.Driver/Types/FixedStringType.cs | Provide both string and byte[] typed representations while preserving boxed mode selection. |
| ClickHouse.Driver/Types/UuidType.cs | Add typed Guid read for fast POCO assignment. |
| ClickHouse.Driver/Types/IPv4Type.cs | Add typed IPAddress read for fast POCO assignment. |
| ClickHouse.Driver/Types/IPv6Type.cs | Add typed IPAddress read for fast POCO assignment. |
| ClickHouse.Driver/Types/BooleanType.cs | Add typed bool read for fast POCO assignment. |
| ClickHouse.Driver/Types/BFloat16Type.cs | Add typed float read for fast POCO assignment. |
| ClickHouse.Driver/Types/Int8Type.cs | Add typed sbyte read for fast POCO assignment. |
| ClickHouse.Driver/Types/Int16Type.cs | Add typed short read for fast POCO assignment. |
| ClickHouse.Driver/Types/Int32Type.cs | Add typed int read for fast POCO assignment. |
| ClickHouse.Driver/Types/Int64Type.cs | Add typed long read for fast POCO assignment. |
| ClickHouse.Driver/Types/UInt8Type.cs | Add typed byte read for fast POCO assignment. |
| ClickHouse.Driver/Types/UInt16Type.cs | Add typed ushort read for fast POCO assignment. |
| ClickHouse.Driver/Types/UInt32Type.cs | Add typed uint read for fast POCO assignment. |
| ClickHouse.Driver/Types/UInt64Type.cs | Add typed ulong read for fast POCO assignment. |
| ClickHouse.Driver/Types/Float32Type.cs | Add typed float read for fast POCO assignment. |
| ClickHouse.Driver/Types/Float64Type.cs | Add typed double read for fast POCO assignment. |
| ClickHouse.Driver/Types/Enum8Type.cs | Add typed string read to avoid boxing for enum materialization. |
| ClickHouse.Driver/Types/Enum16Type.cs | Add typed string read to avoid boxing for enum materialization. |
| ClickHouse.Driver/Poco/PocoReadExpressionFactory.cs | Build per-column, typed read expressions driven by ITypedReader<T>, with Nullable/LowCardinality handling. |
| ClickHouse.Driver/Poco/PocoColumnAssignment.cs | Centralize assignment validation/error text used by both MapTo<T> and fast-path fallback. |
| ClickHouse.Driver/Poco/PocoTypeRegistry.cs | Cache and build per-shape per-column row materializers with mixed fast/fallback/discard behavior. |
| ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs | Expose internal fast-path hooks (TryGetRowMaterializer, TryReadRow) and reuse shared assignment logic. |
| ClickHouse.Driver/ClickHouseClient.cs | Switch QueryAsync<T> to use fast path when available, otherwise retain the existing boxed loop. |
| ClickHouse.Driver.Tests/Copy/PocoReadFastPathTests.cs | Add coverage for parity, multi-type bindings, Nullable/LowCardinality, mixed composite fallback, and failure modes. |
alex-clickhouse
marked this pull request as ready for review
July 20, 2026 11:34
QueryAsync<T> now materializes rows with compiled per-column delegates that read each value straight from the response stream into the target property, bypassing the reader's shared object[] row buffer and the boxing/unboxing MapTo<T> setter. This removes the box (on decode) and the unbox (on assign) per value-type property, per row: ~38% fewer managed allocations and roughly half the Gen0 collections on a 500k-row 3-column read. Mirrors the box-free write path (#434): a new ITypedReader<T> interface, implemented across every scalar/value type; PocoReadExpressionFactory dispatches on it (transparently unwrapping Nullable(T)/LowCardinality(T)); PocoTypeRegistry compiles and caches a per-wire-column delegate array. Columns are mixed per-column: typed box-free read, a boxed fallback (byte-identical to MapTo, with fail-fast validation) for composites, or a discard for unmapped columns. Also adds multiple read representations, selected by the property type (QueryAsync<T> only; MapTo<T> keeps its strict exact-type rules): - DateTime/DateTime64/Date -> DateTime, DateTimeOffset, or DateOnly - String/FixedString -> string or byte[] - Decimal -> decimal or ClickHouseDecimal - Int128/UInt128 -> BigInteger or native System.Int128/UInt128 (net8+) Each type's boxed Read() delegates to its typed ReadValue, so both paths are byte-identical by construction. A read value converter or unregistered type falls back entirely to the existing path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AllScalarsPoco tested Float32/Float64 but not BFloat16 (which also materializes into a float via its own ITypedReader<float>). Add a BFloat16 column using an exactly-representable value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Clearer than TryReadRow (which collided conceptually with the public Read()): the method advances to and materializes the next row into T, pairing with TryGetRowMaterializer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read: QueryAsync<T> can materialize a Map(K,V) column into a List<KeyValuePair<K,V>> or KeyValuePair<K,V>[] property (in addition to Dictionary<K,V>), via a MapType special case in PocoReadExpressionFactory that dispatches to a generic MapMaterializer closed over K,V. The list/array form preserves on-wire entry order and duplicate keys, which a Dictionary collapses. Dictionary properties are unchanged (boxed path). Write: MapType.Write now also accepts an ordered collection of KeyValuePair<,> (e.g. List / array), not just IDictionary — so such a POCO property round-trips into a Map column on binary insert. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Enum8Type and Enum16Type now implement ITypedReader<int> alongside ITypedReader<string>, so a QueryAsync<T> property can bind to either the enum label or the stored numeric value. Both overloads differ only by return type, so they become explicit interface impls sharing a private ReadLabel helper — the pattern StringType and AbstractDateTimeType already use for their multiple representations. The numeric read consumes the same bytes as the label read (1 for Enum8, 2 for Enum16), keeping it byte-identical to the boxed path, and skips the label lookup: the raw value cannot fail on a member absent from the column's enum definition. An int property on an enum column previously threw at plan time, since the boxed fallback yields a label string that is not assignable to int. A Nullable(Enum) column still needs an int? property. Co-Authored-By: Claude <noreply@anthropic.com>
alex-clickhouse
force-pushed
the
poco-read-boxfree
branch
from
July 31, 2026 07:37
c8b211f to
1233440
Compare
Both fixtures already derive from AbstractConnectionTestFixture but rolled their own CreateTestTableName, which predates #470. Switch to the inherited CreateTableName(): it produces the same randomized, TFM-tagged name and registers it for the fixture's OneTimeTearDown drop, so the tables no longer leak on the shared server. Also drops the scaffolding #470 made unnecessary once names are unique: the CREATE TABLE IF NOT EXISTS (plain CREATE now fails loudly on a naming regression instead of silently reusing a table) and the try/finally whose only job was the DROP TABLE IF EXISTS. The insert still receives the bare name, via TestUtilities.BareTableName, because InsertOptions.Database is what resolves the table in that assertion -- passing a qualified name there would make the option a no-op.
Line coverage read 95.5% on PocoReadExpressionFactory but hid the real surface: TryBuildTypedRead is a single Expression.Call, so one scalar test marked it covered while ~15 of the ITypedReader<T> implementations it can dispatch to were never reached — FixedString, UUID, IPv4/IPv6, Time/Time64, native UInt128, all four BigInteger readers, ClickHouseDecimal, and the plain DateTime representation across the whole date/time family. Add PocoReadFastPathParityTests (server-free), split by what is not already covered elsewhere: - dispatch: that a (column type, CLR target) pair resolves to a typed read of exactly that type. Novel — nothing else asserts it, and since most types' typed and boxed reads are the same method (Int8Type.Read => ReadValue), a type silently losing its ITypedReader<T> would still yield correct values via the fallback. - decode + alignment: only for representations the boxed reader cannot produce (byte[], DateOnly/DateTimeOffset, native Int128/UInt128, ClickHouseDecimal, Map-as-pairs, the Nullable lift). The rest are already round-tripped by SerialisationTests and read off a real server by SqlSimpleSelectTests via TestCases.GetDataTypeSamples(). Each case reads a trailing sentinel, so a reader consuming the wrong byte count fails here rather than corrupting the next column of a real row. - negative: pairs that must decline and fall back to the boxed reader, including SimpleAggregateFunction, which is wire-transparent like LowCardinality but is not unwrapped by the factory. Extend PocoReadFastPathTests with the fast-path-only representations over a real server, and cover PocoTypeRegistry.BuildBoxedColumnReader's null/DBNull and InvalidCastException branches plus PocoColumnAssignment's polymorphic accept — a second copy of MapTo's assignment rules that had no direct tests. Assert DateTimeKind explicitly (DateTime.Equals ignores it, so the previous literals were wrong but passing) and ClickHouseDecimal.Scale (Equals rescales before comparing), and use byte-asymmetric wide-integer values: UInt128.MaxValue is all 0xFF, so it cannot detect a byte-order flip. Coverage: PocoReadExpressionFactory 95.5% -> 100%, PocoColumnAssignment 93.3% -> 100%, PocoTypeRegistry 95.6% -> 99.3%. Verified by mutation — disabling the fast path fails 96 of 134 tests. Tests only; no driver source changes. Co-Authored-By: Claude <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
PocoTypeRegistry caches compiled row readers under the column name plus the column type's ToString(), and each compiled reader closes over the label map of the enum instance it was built from (baked in via Expression.Constant). Enum16Type overrode ToString() to a bare "Enum16", omitting the members, so two Enum16 columns sharing a name but not their definitions produced the same cache key. The second query then reused the delegate built for the first and reported that column's labels — silently wrong, or a KeyNotFoundException when the stored value was absent from the older map. The boxed Read + MapTo path was correct, so the two disagreed. Drop the override so Enum16 inherits EnumType.ToString(), which lists the members. Enum8Type never overrode it and was unaffected; the enum -> int read has no label dependence and was unaffected either way. Enum types register via RegisterParameterizedType, which keys off Name rather than ToString(), so registration and parsing are unchanged. The override dates to the 2020 formatters refactor that replaced TypeCode with ToString (7629397) and looks like a leftover rather than a decision — Enum8Type got the same treatment and kept the inherited form. Reported by Cursor Bugbot on #449, which also named Enum8Type; only Enum16Type actually overrode ToString(). Regression test covers both widths. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e282292. Configure here.
…arser
EnumType.ToString() built its output as
$"{Name}({string.Join(",", Values.Select(kvp => kvp.Key + "=" + kvp.Value))}"
which is malformed three ways: the closing parenthesis is missing, labels are
not quoted, and they are not escaped. An Enum8('a' = 1, 'b' = 2) column
rendered as "Enum8(a=1,b=2".
That output is not merely cosmetic. SchemaDescriber writes it into
GetSchemaTable()'s ProviderType column and re-parses that value, it is the type
name handed to a custom IParameterFormatter, it appears in column-mapping error
messages, and PocoTypeRegistry keys its row-reader cache on it. Re-parsing the
rendered form threw, and for a label containing a comma it silently produced
different members: 'a,b' = 1 rendered as "a,b=1" and split into two.
Render the ClickHouse declaration form instead — Enum8('a' = 1, 'b' = 2) —
quoting each label and escaping backslash and quote, the inverse of the
Regex.Unescape that ParseEnumMember applies. Parsing is untouched, and enum type
registration keys off Name rather than ToString(), so lookup is unaffected.
Adds a round-trip test over labels containing a comma, an equals sign, an
escaped quote and a backslash, plus one pinning the exact rendered form. Seven
of the resulting cases fail without this change.
Follow-up to e282292, which removed Enum16Type's bare "Enum16" override and so
made Enum16 inherit this rendering too.
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes
ClickHouseClient.QueryAsync<T>materialize rows without per-value boxing, and adds multiple read representations per column. Mirrors the box-free write path (#434).Today the POCO read path boxes twice per value-type cell: once in
Read()(into the sharedobject[]row buffer) and once unboxing in the compiledAction<T,object>MapTo<T>setter.QueryAsync<T>now compiles a per-column delegate that reads each value straight from the stream into the strongly-typed property, eliminating both.How
ITypedReader<T>(analog ofITypedWriter<T>) implemented across every scalar/value type. Each type's boxedRead()delegates to its typedReadValue, so the two paths are byte-identical by construction.PocoReadExpressionFactorydispatches viaITypedReader<targetClrType>, transparently unwrappingNullable(T)andLowCardinality(T).PocoTypeRegistrycompiles + caches a per-wire-column delegate array, keyed by(T, wire-column signature). Per-column mixing: box-free typed read, a per-column boxed fallback for composites (byte-identical toMapTo, with fail-fast validation), or a discard for unmapped columns — so one composite column doesn't disable the fast path for its scalar siblings.PocoColumnAssignmentkeepsMapToand the fallback's validation/errors identical.Multiple read representations (selected by property type;
QueryAsync<T>only —MapTo<T>keeps strict exact-type rules)DateTime/DateTime64/DateDateTime,DateTimeOffset,DateOnlyString/FixedStringstring,byte[]Decimaldecimal,ClickHouseDecimalEnum8/Enum16string(label),int(stored numeric value)Int128/UInt128BigInteger, or nativeSystem.Int128/UInt128(net8+)Results (
PocoReadBenchmark,long/string/double, local WSL)GetValue/GetFieldValue<T>(ADO contract) unchanged.Testing
PocoReadFastPathTests(23 tests): all-scalar parity vs the boxed path, every multi-type binding, Nullable variants, LowCardinality, per-column mixing with composite fallback, unmapped-column alignment, fail-fast validation, unregistered-type error.Enum8/Enum16read as both label andintover the same column (negative and beyond-sbytevalues pin each wire width),Nullable(Enum8)intoint?, anint-property insert round-tripping through a realEnum8column, and the fail-fast error for a non-nullableinton aNullable(Enum)column (which needsint?).