TCP N9: POCO row insert — InsertRowsAsync<T> over a compiled per-column gather - #559
Open
alex-clickhouse wants to merge 7 commits into
Open
TCP N9: POCO row insert — InsertRowsAsync<T> over a compiled per-column gather#559alex-clickhouse wants to merge 7 commits into
alex-clickhouse wants to merge 7 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the TCP client’s row-oriented insert half, complementing #557’s POCO query support.
Changes:
- Adds POCO and positional
object[]insert overloads. - Compiles cached per-column gather plans with codec-aware null/type handling.
- Preserves connections across mapping failures and adds broad unit/integration coverage.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
ClickHouse.Driver.Tcp/Types/Codecs/NullableColumnCodec.cs |
Exposes lifted writable types. |
ClickHouse.Driver.Tcp/Types/ArrayColumn.cs |
Adds pooled-buffer ownership. |
ClickHouse.Driver.Tcp/Protocol/ClickHouseTcpConnection.cs |
Adds schema-driven column factories. |
ClickHouse.Driver.Tcp/Poco/PocoWritePlan.cs |
Builds compiled POCO write plans. |
ClickHouse.Driver.Tcp/Poco/PocoWriteConversion.cs |
Resolves property-to-codec conversions. |
ClickHouse.Driver.Tcp/Poco/PocoUntypedColumns.cs |
Transposes positional rows. |
ClickHouse.Driver.Tcp/Poco/PocoTypeRegistry.cs |
Caches write plans. |
ClickHouse.Driver.Tcp/Poco/PocoTypeDescriptor.cs |
Shares mapped-column descriptions. |
ClickHouse.Driver.Tcp/Poco/PocoRowBuffer.cs |
Materializes row sources. |
ClickHouse.Driver.Tcp/Poco/PocoReadPlan.cs |
Uses shared block signatures. |
ClickHouse.Driver.Tcp/Poco/PocoColumnBuilder.cs |
Compiles per-column gathers. |
ClickHouse.Driver.Tcp/Poco/PocoBlockSignature.cs |
Centralizes plan cache keys. |
ClickHouse.Driver.Tcp/Client/IClickHouseTcpClient.cs |
Adds row-insert contracts. |
ClickHouse.Driver.Tcp/Client/ClickHouseTcpClient.cs |
Implements row inserts. |
ClickHouse.Driver.Tcp.Tests/Types/NullableColumnCodecTests.cs |
Tests lifted nullable writes. |
ClickHouse.Driver.Tcp.Tests/Types/ArrayColumnTests.cs |
Tests buffer ownership. |
ClickHouse.Driver.Tcp.Tests/Protocol/ClickHouseTcpConnectionInsertTests.cs |
Tests factory lifecycle. |
ClickHouse.Driver.Tcp.Tests/Poco/PocoWritePlanTests.cs |
Tests write-plan behavior. |
ClickHouse.Driver.Tcp.Tests/Poco/PocoUntypedColumnsTests.cs |
Tests positional transposition. |
ClickHouse.Driver.Tcp.Tests/Poco/PocoRowBufferTests.cs |
Tests row materialization. |
ClickHouse.Driver.Tcp.Tests/Integration/PocoWriteIntegrationTests.cs |
Exercises real-server round trips. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
August 16, 2026 10:32
aa3978d to
62949f6
Compare
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
August 16, 2026 17:51
62949f6 to
4283c67
Compare
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
August 17, 2026 09:49
4283c67 to
3358c02
Compare
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
August 17, 2026 14:00
3358c02 to
2a81c80
Compare
14 tasks
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
August 18, 2026 06:49
906b78b to
6ef885e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
August 18, 2026 07:29
6ef885e to
812c488
Compare
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
August 18, 2026 08:11
812c488 to
7343f2f
Compare
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
2 times, most recently
from
August 22, 2026 16:47
5f06f3c to
c3fbb47
Compare
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
3 times, most recently
from
August 26, 2026 08:59
ec292d5 to
997f885
Compare
alex-clickhouse
marked this pull request as ready for review
August 28, 2026 12:06
The write half of Branch 2 (N9), stacked on the read half. A row-oriented insert transposes the caller's rows into the columnar insert the connection already sends, in both shapes: `InsertAsync<T>(IEnumerable<T>)` gathering each property into the buffer its target column is written from, and `InsertAsync(IEnumerable<object[]>)` matching values to targets by position. The gather is the mirror of the read scatter: one compiled loop per target column, no boxing and no per-row delegate hop. It needs no conversion layer of its own — a codec already accepts the calendar types on write, so the only conversions left are the CLR-level ones a cast would do (nullable lift, enum ordinal, reference upcast). Numeric widening is declined in both directions, so every shape that inserts also reads back. Whether a row may have no value for a column is the codec's question, not the CLR's: `NullPlaceholder is null` is true exactly for the types with a NULL of their own, so a null `string` property into a plain `String` column is reported before anything is sent, naming the row, rather than faulting inside the codec part-way through a block and taking the connection with it. The target types arrive in the server's sample block, after the statement has gone out, so the connection gains an InsertColumnFactory seam that builds the columns there and owns them afterwards. A factory that throws — a mapping error, which is the caller's shape rather than the connection's — closes the row stream with no rows and reports once the connection is back to Ready, exactly as a schema mismatch does. Also lifts `Nullable`'s WritableElementTypes to its nullable surface. CanWrite has always accepted `DateTime?` for a `Nullable(DateTime)` column, but the list reported only the canonical `uint?`, which a plan choosing a write type from the list rather than probing with a column cannot see. Co-Authored-By: Claude <noreply@anthropic.com>
The check sat at the buffer's growth points, which a counted source never reaches — it rents once to fit — so a long `List<T>` was drained in full whatever the token said. Tested per row instead, next to the null-row check: the read is a field test against a token that is usually None, so it costs nothing measurable beside the source's own MoveNext. Co-Authored-By: Claude <noreply@anthropic.com>
The corpus insert test knows a Nested target cannot be gathered from rows and asserts the refusal instead of a successful insert. It recognised the shape with StartsWith, so it only caught a top-level Nested -- and the corpus also has Array(Nested(a UInt8)), Tuple(Nested(a UInt8), String) and Nested in both Map positions. Those four expected an insert that cannot work, and failed on every framework and every server version. Contains, not StartsWith: a composite can only hand its child the column shape a row yields, so a Nested inside one is exactly as ungatherable, and refuses for the same reason with the same message. This mirrors b75f932 on tcp/epic-b9-tls, which made the codec itself refuse a Nested inside a composite rather than only a top-level one. That commit is on a different epic line and never reached this branch, so the test kept the narrow check. Co-Authored-By: Claude <noreply@anthropic.com>
A row insert compiled its property mapping and then gathered every row of every column before it sent a byte, so the conversion held one buffer per column of the whole insert's row count: for 105 columns and 100k rows, 47 MB of large-object-heap arrays, and a column-major pass over a row set far larger than cache. Make the wire block the conversion unit. The insert factory now returns an IInsertColumnSource: it compiles the mapping once against the server's schema, rents one gather buffer per target column sized for a single block, and refills them for each block in turn. The columns keep their identity as they are refilled, so the insert plan is built once and the block loop only asks for the next range of rows. The compiled gather takes a range rather than a count, and carries the row number the range starts at so an error still names the row by its place in the insert. PocoRowBuffer stages a non-array list through a block-sized window for the same reason, instead of copying every row reference up front. Cap a block at fifty thousand rows, which now bounds the conversion as well as the wire. Two consequences to know. A value the target cannot take is found when its own block is converted, so the blocks before it have been sent and the server keeps them; the row stream still closes cleanly and the connection stays reusable. And the caller's list is read as the insert runs, which the documented "do not modify the rows until it completes" rule already covers. Measured, 100k rows x 105 columns, ENGINE Null, median of 5 post-warmup rounds, block caps interleaved within one process: cap before after one block 171.5 ms 179.4 ms 50,000 191.0 ms 163.6 ms 8,192 183.9 ms 149.1 ms 1,024 214.1 ms 151.2 ms Live large-object bytes after the insert, one configuration per process: 123.3 MB for the old default, 94.0 MB for the new one, and 24.5 MB at 8,192 rows a block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The buffered writer never flushes on its own: the codec write path is synchronous all the way down, so the only place an insert can reach the socket is the check between two columns of a block. Everywhere else a write that does not fit grows the buffer by doubling, and every size it passes through goes back to the array pool and stays there. With the cap at 10 MiB that ladder climbs to 16 MiB, and the pool keeps every rung. Measured on 100k rows x 105 columns, live large-object bytes after the insert fall from 94 MB to 34 MB when the cap is 1 MiB, with no change in wall clock. A quarter of a megabyte saves nothing more, so the gather buffers are what is left. Start the buffer at 64 KiB rather than 16 KiB, the largest array-pool bucket that stays off the large-object heap, so an insert reaches the cap in four doublings instead of six. Take the client's default from BlockWriter's constant so the two cannot drift: the internal fallback stood at 50 MiB while every insert through the client passed 10 MiB, which reads as a 50 MiB default to anyone following the parameter. The cap remains a soft one. It is checked after a whole column, so peak buffered bytes are the cap plus the column that crossed it, and a column larger than the cap buffers in full. Rows per block is what bounds that column, and the docs now say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
alex-clickhouse
force-pushed
the
tcp/epic-n9-poco-write
branch
from
September 3, 2026 09:25
c9c2555 to
29d5215
Compare
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.
Stacked on #557 (
tcp/epic-n5-poco-read) — review that one first; this PR's diff is the write half only.TCP epic N9: row-oriented insert, both shapes. The read half of Branch 2 landed in #557; this is the mirror, so a POCO now round-trips through the native protocol.
What it does
InsertRowsAsync<T>(IReadOnlyList<T>)— one compiled gather per target column, each pulling one property out of every row into the buffer that column is written from. Box-free (except aVariant/Dynamictarget, written fromobject), no per-row delegate hop, and for a fixed-width column the gathered buffer reaches the wire as a single blit. Names match as the read side's do: case-, then underscore-insensitive, with[ClickHouseTcpColumn]/[ClickHouseTcpNotMapped].InsertRowsAsync(IReadOnlyList<object[]>)— the dynamic tier, positional by the sample block's order. Each column's CLR type comes from the first row that has a value there, so hand-writtenDateTimevalues and the raw epoch seconds the untyped read produces are both accepted, and a read-then-reinsert needs no conversion by the caller.Arrays and
List<T>both pass naturally. Arrays are borrowed for the operation; otherIReadOnlyList<T>inputs are shallow-copied once into pooled storage so the compiled gathers retain an array fast path. Callers therefore materialize lazy sources before inserting, and must not mutate rows until the operation completes.No conversion layer of its own. The read side must ask the codec for its conversions, because a column decodes to the raw wire value. Write does not: a codec already accepts
DateTime/DateTimeOffset/TimeSpandirectly, so the only conversions left are the CLR-level ones a cast would do — nullable lift, enum ordinal, reference upcast. Numeric widening is declined in both directions, which is what keeps "inserts" and "reads back" the same set of shapes.Decisions worth a look
InsertColumnFactoryseam. A factory that throws parks its exception, closes the row stream with no rows, drains, and rethrows once the connection is back toReady— the course the schema-mismatch path already took. The columns a factory returns are the insert's to dispose; a caller's own columns are untouched, as before.NullPlaceholder is nullis true exactly forNullable, a nullableLowCardinality,VariantandDynamic. Asking the CLR instead (default(TWrite) is null) let a nullstringproperty into a plainStringcolumn through the gather, to fault insideWriteColumnpart-way through a block — which terminates the connection. The gather compiles a null test for reference-typed properties too, so it fails before anything is sent, naming the row.Nullable'sWritableElementTypeswas under-reporting.CanWritehas always acceptedDateTime?for aNullable(DateTime)column, but the list reported only the canonicaluint?— invisible to a caller who probes with a column, fatal to one that picks a type from the list. Now lifted, along withNullPlaceholderAs.UnspecifiedDateTimenow denotes the same session wall clock on write that the read path presents, and the POCO plan cache includes that timezone in its key.InsertRowsAsyncleavesInsertAsyncexclusively columnar, so external concreteIColumn<T>[]implementations and column lists do not collide with a generic row overload.Known follow-up
LowCardinality(DateTime)reads asDateTimebut is written only fromuint. It lifts its inner's readable types and not its writable ones, so it is the last codec whose two surfaces disagree. Pre-existing, and the columnar path has always had it, but the POCO layer is what makes it visible. Documented inPocoWriteConversionand left as a follow-up, since closing it means giving theLowCardinalitywrite path a shape per write type asNullablehas.Testing
2,124 tests pass on net9 against a real server. Overall TCP coverage is 93.94% line / 88.24% branch / 95.52% method; every changed executable line is covered, and the POCO row-buffer path remains at 100% line and branch coverage.
InsertRoundTripCasecases are inserted asRow<T>and read back.Nestedis the one shape rows cannot fill — its writer needs its own column type — so the corpus test asserts the refusal for it rather than skipping.DateTimeandDateTime64(3)withsession_timezone = Europe/Amsterdamthrough columnar, POCO, and untyped row inserts. The columnar case uses an external-style concreteIColumn<T>[], compile-pinning the overload-resolution fix.Readyrather than being redialled, plus the factory's ownership (dispose count) and that the factory's own exception comes back with its identity intact.Release builds pass for net8.0, net9.0, and net10.0.
No CHANGELOG/RELEASENOTES entry: no TCP epic PR has one, the assembly being unreleased and
[Experimental]. The epic gets a single entry when it ships.