Skip to content

TCP N9: POCO row insert — InsertRowsAsync<T> over a compiled per-column gather - #559

Open
alex-clickhouse wants to merge 5 commits into
tcp/epic-n5-poco-readfrom
tcp/epic-n9-poco-write
Open

TCP N9: POCO row insert — InsertRowsAsync<T> over a compiled per-column gather#559
alex-clickhouse wants to merge 5 commits into
tcp/epic-n5-poco-readfrom
tcp/epic-n9-poco-write

Conversation

@alex-clickhouse

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

Copy link
Copy Markdown
Collaborator

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.

await client.InsertRowsAsync("INSERT INTO t (id, user_name) VALUES", accounts);      // POCO rows
await client.InsertRowsAsync("INSERT INTO t (id, user_name) VALUES", objectRows);    // untyped, positional

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 a Variant/Dynamic target, written from object), 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-written DateTime values 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; other IReadOnlyList<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/TimeSpan directly, 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

  • Every target column must map to a property, unlike a read, where an unmapped column is skipped: the server expects a value for each column of the statement's list. The remedy the message names is the statement's own column list, which is also how one POCO fills part of a table. A property with no column stays silent.
  • A mapping error lands mid-INSERT and must not cost the connection. The target types only arrive in the sample block, so the columns are built there, behind a new internal InsertColumnFactory seam. A factory that throws parks its exception, closes the row stream with no rows, drains, and rethrows once the connection is back to Ready — 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.
  • "Can this column hold a null" is the codec's question. NullPlaceholder is null is true exactly for Nullable, a nullable LowCardinality, Variant and Dynamic. Asking the CLR instead (default(TWrite) is null) let a null string property into a plain String column through the gather, to fault inside WriteColumn part-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's WritableElementTypes was under-reporting. CanWrite has always accepted DateTime? for a Nullable(DateTime) column, but the list reported only the canonical uint? — invisible to a caller who probes with a column, fatal to one that picks a type from the list. Now lifted, along with NullPlaceholderAs.
  • Timezone-less calendar writes use the operation's sample context. The columnar, POCO, and untyped planners all resolve target codecs through the sample block's registry and session/server timezone. An Unspecified DateTime now denotes the same session wall clock on write that the read path presents, and the POCO plan cache includes that timezone in its key.
  • Rows have a dedicated method name. Keeping row inserts under InsertRowsAsync leaves InsertAsync exclusively columnar, so external concrete IColumn<T>[] implementations and column lists do not collide with a generic row overload.

Known follow-up

LowCardinality(DateTime) reads as DateTime but is written only from uint. 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 in PocoWriteConversion and left as a follow-up, since closing it means giving the LowCardinality write path a shape per write type as Nullable has.

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.

  • Per-type coverage rides the corpus: all 203 InsertRoundTripCase cases are inserted as Row<T> and read back. Nested is 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.
  • POCO→POCO round trip, calendar and enum properties (including the nullable spellings), an insert-only immutable POCO, a narrower column list, a materialized 5,000-row list across several blocks, and the untyped path's positional round trip and read→re-insert.
  • Real-server regressions cover timezone-less DateTime and DateTime64(3) with session_timezone = Europe/Amsterdam through columnar, POCO, and untyped row inserts. The columnar case uses an external-style concrete IColumn<T>[], compile-pinning the overload-resolution fix.
  • The mid-INSERT refusals all assert the client is still usable afterwards, and the connection-level tests pin that the connection itself stays Ready rather than being redialled, plus the factory's ownership (dispose count) and that the factory's own exception comes back with its identity intact.
  • Unit tests cover only what a round trip cannot see: write-type selection, plan-build refusals, cache keys, array borrowing, list copying, null validation, cancellation, and pooled-buffer ownership.

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.

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 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.

Comment thread ClickHouse.Driver.Tcp/Poco/PocoRowBuffer.cs Outdated
@codecov

codecov Bot commented Aug 18, 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-n9-poco-write branch 2 times, most recently from 5f06f3c to c3fbb47 Compare August 22, 2026 16:47
@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-n9-poco-write branch 2 times, most recently from 385deb2 to ec292d5 Compare August 22, 2026 17:25
alex-clickhouse and others added 3 commits August 26, 2026 10:51
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>
@alex-clickhouse alex-clickhouse changed the title TCP N9: POCO row insert — InsertAsync<T> over a compiled per-column gather TCP N9: POCO row insert — InsertRowsAsync<T> over a compiled per-column gather Aug 28, 2026
@alex-clickhouse
alex-clickhouse requested a balanced review from Copilot August 28, 2026 12:05
@alex-clickhouse
alex-clickhouse marked this pull request as ready for review August 28, 2026 12:06

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 29 out of 29 changed files in this pull request and generated 2 comments.

Comment thread ClickHouse.Driver.Tcp/Types/Codecs/NullableColumnCodec.cs

@kavirajk kavirajk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM 👍

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.

3 participants