TCP converters 3/6: POCO reads on the fused converter loop by alex-clickhouse · Pull Request #670 · ClickHouse/clickhouse-cs

alex-clickhouse · 2026-10-09T22:42:56Z

This is part 3 of the stack of pull requests for ClickHouse/integrations#801, a converter layer for the native TCP
client. It is stacked on #668 (part 2b, Block.ReadAs<T> and CanRead on the converter layer), which is on #667, #666
and #665. It moves POCO mapping (QueryAsync<T>) onto the converter derivation. What QueryAsync<T> accepts, the values
that it gives and its error messages do not change.

What

For each column of a result, the POCO read plan derives the converter tree that reads the column type as the type of
its property, with the read rules of POCO mapping (a CLR enum from its ordinal, a cast that the CLR allows,
a nullable target, and a nullable column read as a value type that throws at the first NULL).
Where the runtime compiles expression trees, the plan splices the tree into one compiled loop for each column, which
sets the property of each row (PocoScatterTier.Emit).
Where the runtime does not compile dynamic code (RuntimeFeature.IsDynamicCodeCompiled is false), the plan reads the
rows with the tree's bulk read into a pooled buffer and sets each property through a setter delegate
(PocoScatterTier.Fill). That tier builds no expression from the tree, because the expression interpreter cannot run
the span locals of the trees.
A LowCardinality reader keeps the converted dictionary of each column in a weak table that is safe for concurrent use,
so the 256-row windows of one block convert the dictionary once, also for a dictionary inside an Array or a Map.
Variant, Dynamic, Nested, QBit and the geo types read through the indexer of their columns, so a window of rows
makes only the values of that window, as before. Their Values would make the values of the whole block into a cache.

How

PocoColumnScatterFactory keeps the old checks and texts: a column that does not surface its codec's element type, and
a property type that the column cannot be read as (with the types that it reads as). The texts are in
PocoReadErrors.
A NULL that a read rule meets throws an internal exception that holds the row of the column. The scatter throws the
old exception for it, with the row of the result (rowOffset + row - start).
Rows are still built with the compiled constructor of the POCO type.
The old scatter moves unchanged to the internal LegacyPocoColumnScatterFactory, which only the tests call, through
PocoReadPlan.BuildLegacy. It is the reference of the differential tests. The last part of the stack deletes it with
PocoValueProjection.

Why

The POCO tier had its own copy of the conversion rules (PocoValueProjection and the codecs' projections). With the
converter derivation, QueryAsync<T>, Block.ReadAs<T> and CanRead use one set of rules. The compiled loop also makes
the composite and LowCardinality POCO reads faster (see Benchmarks).

What a user sees

No change in behavior: the same accepted readings, values, error messages and order of failures, in both tiers. A
property setter that throws gives the same first failure in row order as before: when the bulk read of a window fails,
the Fill tier reads and sets the rows of that window again one at a time. There is no changelog fragment and no docs
change, because nothing visible changes.

Tests

The differential tests of part 0 run the client's POCO plan for every POCO facet of the case list (567), once with
the tier that the runtime chooses and once with the Fill tier forced, against the old plan, for all rows and for a
tail that starts above zero. No change is declared. The reference outcome counts do not change.
PocoReadRulesTests compares both tiers with the old plan for 36 column types and 53 targets (enums, casts to
object and to interfaces, nullable targets, tuples, maps, the array casts), for all rows and for a window that starts
past the first row of the block and of the result: the same refusal, values, or failure with the same inner exception.
PocoReadPlanFillTests and PocoReadFillIntegrationTests run the POCO plan tests and the server-backed POCO read
tests again with the Fill tier forced. The second does not run in the Cloud job: it is coverage, not Cloud behavior.
PocoReadNoDynamicCodeTests starts the test assembly in a child process whose runtime configuration turns dynamic code
off (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported: false, which the DynamicCodeSupport
MSBuild property sets). There the plan chooses the Fill tier by itself and Expression.Compile interprets the
constructor of the rows. The child reads twelve column types (LowCardinality, Nullable, Array, Map, Tuple, Variant and
the read rules) and a NULL into a value type, and gives the rows and the failure text of the compiled loop. For this
test, the test project has its own entry point, which the test runner does not call. The test kills the child process
if it does not end in 2 minutes.
New unit tests: the dictionary of a column converted once for all windows (Emit, Fill, inside an Array, the lifting
dictionary, two columns, eight threads at once, through the scatter of each tier, and a column that nothing else holds
is not kept alive); the indexer reads of the canonical-only types; the tier choice; the Fill tier with a tree that has
no expression; a property setter that throws and a NULL, in both orders, through the old plan and both tiers.
Tests changed on purpose:
- PocoReadPlanTests: ten tests read columns that a codec decoded where a converter needs the decoded shape of the
  column type (the Nullable, Array and String columns); the assertions do not change. A new test builds a
  Nullable(Int32) column without that shape and pins the failure, which names the column and the missing shape. Only
  tests can build such a block: Block has only an internal constructor. The tests read in the tier of their fixture,
  and the window test is one test in each fixture instead of two cases.
- Moved, so that they run once: Materialize_EveryTier_ProducesTheSameRows and
  ReadPlanFor_DifferentForcedTiers_CompileTheirOwnPlans to PocoColumnScatterFactoryTests, and the integration test
  Materialize_EveryScatterTier_ReadsTheSameRows to PocoReadTierIntegrationTests; they name the new tiers. The test
  of the old indexer tier under the expression interpreter and the three tests of the old tier choice move to
  LegacyPocoColumnScatterFactoryTests, unchanged except for the names of the legacy types.
- The tests of PocoReadIntegrationTests move unchanged to an abstract base class, PocoReadIntegrationTestBase, which
  creates its client through a virtual method, so that the Fill fixture can force the tier. The names of the tests do
  not change.
- The read-rule matrix test and the two LowCardinality failure-order tests of part 2a compare with the old POCO plan.
net9.0, full TCP suite with a server: 5378 passed on part 2b, 5773 passed here (+395), 0 failed, 0 skipped.
net8.0 and net10.0, the offline suite (no server): 3481 passed each.
The public API does not change.

Benchmarks

The performance gate of the stack: the converter benchmarks of part 0 on part 0 (base) and on this part (head), run
alternately in 4 rounds in one quiet window on the same machine. A shape fails when its ratio head/base is above 1.05 in
every pair. Measured on 015f7c2d; the later commits change only tests, comments and the failure path of the Fill tier.

base 77ca7eeb, head 015f7c2d, 4 rounds (base then head in each), limit 1.05
  round 1: base load average 2.02 to 3.09, CPU busy before 13% and after 12%; head load average 2.84 to 2.63, CPU busy before 11% and after 13%
  round 2: base load average 2.42 to 3.17, CPU busy before 14% and after 11%; head load average 2.91 to 3.22, CPU busy before 12% and after 12%
  round 3: base load average 2.59 to 2.58, CPU busy before 13% and after 11%; head load average 2.45 to 3.00, CPU busy before 16% and after 13%
  round 4: base load average 2.76 to 3.32, CPU busy before 12% and after 13%; head load average 3.14 to 2.57, CPU busy before 13% and after 12%

shape                                                                   base ms  ratio  pairs            spread base/head  verdict
TcpConverterRead.Columnar(ArrayStringAsBytes)                             0.553  0.706  0.728 0.686 0.684 0.726     9.6%/  6.1%  ok
TcpConverterRead.Columnar(DateTimeAsOffset)                               0.267  0.707  0.714 0.698 0.700 0.723     2.8%/  2.6%  ok
TcpConverterRead.Columnar(LowCardinalityNullableStringAsString)           0.031  1.164  1.219 1.149 1.179 1.127     3.3%/  5.2%  FAIL
TcpConverterRead.Columnar(LowCardinalityStringAsBytes)                    0.112  0.328  0.323 0.332 0.337 0.316     2.7%/  4.3%  ok
TcpConverterRead.Columnar(NullableDateTimeAsOffset)                       0.301  0.626  0.620 0.631 0.639 0.612     2.3%/  2.5%  ok
TcpConverterRead.Columnar(StringAsString)                                 0.226  1.026  1.038 1.024 1.027 1.017     3.1%/  0.9%  ok
TcpConverterRead.Columnar(Wide)                                           1.419  0.793  0.790 0.781 0.810 0.795     4.4%/  4.5%  ok
TcpConverterRead.Poco(ArrayStringAsBytes)                                 0.681  0.670  0.699 0.676 0.665 0.653     5.6%/  9.4%  ok
TcpConverterRead.Poco(DateTimeAsOffset)                                   0.278  1.034  1.041 1.003 1.032 1.035     4.0%/  2.4%  ok
TcpConverterRead.Poco(LowCardinalityNullableStringAsString)               0.259  0.454  0.490 0.430 0.442 0.466     4.7%/  9.3%  ok
TcpConverterRead.Poco(LowCardinalityStringAsBytes)                        0.211  0.552  0.583 0.549 0.555 0.549     2.4%/  4.5%  ok
TcpConverterRead.Poco(NullableDateTimeAsOffset)                           0.291  0.905  0.870 0.912 0.964 0.899     3.7%/  7.2%  ok
TcpConverterRead.Poco(StringAsString)                                     0.347  0.954  0.953 0.907 0.955 0.972     7.7%/  5.2%  ok
TcpConverterRead.Poco(Wide)                                               1.528  0.752  0.750 0.755 0.753 0.750     2.5%/  2.5%  ok
TcpConverterWrite.Insert(ArrayStringFromText)                             0.643  0.972  0.997 1.014 0.947 0.942     5.5%/  4.9%  ok
TcpConverterWrite.Insert(LowCardinalityFixedStringBytesHighRepeat)        0.361  0.989  1.007 0.969 0.972 1.021     3.8%/  3.6%  ok
TcpConverterWrite.Insert(LowCardinalityFixedStringBytesLowRepeat)         1.283  1.001  0.966 1.035 1.011 0.991     3.5%/  3.6%  ok
TcpConverterWrite.Insert(LowCardinalityStringHighRepeat)                  0.308  0.997  0.994 0.730 0.999 1.009    35.7%/  1.8%  ok
TcpConverterWrite.Insert(LowCardinalityStringLowRepeat)                   1.262  0.982  0.949 1.003 0.999 0.965     6.1%/  2.8%  ok
TcpConverterWrite.Insert(NullableDateTimeFromOffset)                      0.157  1.012  1.041 1.633 0.982 0.966     3.8%/ 64.8%  ok

largest run-to-run spread on one side: 64.8%
gate: FAIL (1 shapes slower than 1.05 in every pair)

The POCO read shapes are 0.45x to 0.91x for the composites and the LowCardinality columns, and 0.95x to 1.03x for the two
leaf shapes, which do the same work for each row as before. The columnar shapes that convert are 0.33x to 0.79x, and the
write shapes do not change (0.97x to 1.01x).

The one FAIL, Columnar(LowCardinalityNullableStringAsString) at 1.16x, is not a cost of this part. That shape reads a
LowCardinality(Nullable(String)) column as string, which ReadAs returns as the column itself, and this part does not
change that code. The benchmarks run with tiered compilation off, so the JIT compiles each method once, at its first
call, at the next free place in its code memory. The benchmark's setup builds the POCO read plan for every shape, also
for the columnar ones, and with this part that compiles more methods before the measurement. So the method that reads the
column (NullableLowCardinalityReferenceColumn<T>.Values) starts at another 64-byte offset, and its 10,000-row loop,
which the JIT does not align because it has a call, covers three 64-byte lines of code where it covered two:

The disassembly of that method, and of StringColumn.Values, is the same with and without the setup that slows the
read. Only the embedded addresses differ.
In a separate test program, one extra small method compiled before the measurement moves the method by 32 bytes and
the read from 37.4 µs to 33.1 µs a block, with no code of this part; two extra methods move it back, and it is slow
again. In the benchmark's own processes, the method starts at offset 32 (mod 64) on part 0 and at 0 on this part.
With tiered compilation on (the default runtime mode), base and head interleaved, 10 runs each: 32.6 µs and 33.1 µs a
block (1.02x), within the run-to-run spread.

Decision: the shape is a code-placement artifact of the benchmark setup, not a cost of this part, and the benchmark does
not change.

🤖 Generated with Claude Code

codecov · 2026-10-10T09:27:10Z

Codecov Report

❌ Patch coverage is 96.74419% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...kHouse.Driver.Tcp/Poco/PocoColumnScatterFactory.cs	93.50%	4 Missing and 1 partial ⚠️
....Driver.Tcp/Poco/LegacyPocoColumnScatterFactory.cs	96.92%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Copilot

🟡 Changes recommended

The test project remains a library, so its new child process cannot invoke Program.Main.

1 open finding

Set test project OutputType to Exe for child process entry point · New

What changed in this PR

Moves native TCP POCO reads onto the shared converter layer while preserving existing behavior.

Changes:

Adds compiled Emit and dynamic-code-free Fill scatter tiers.
Caches LowCardinality dictionary conversions and adds canonical indexer reads.
Expands differential, concurrency, integration, and fallback-path coverage.

File	Description
`ClickHouse.Driver.Tcp/Types/Converters/IndexedReader.cs`	Adds canonical indexer reader.
`ClickHouse.Driver.Tcp/Types/Converters/DictionaryReaders.cs`	Caches converted dictionaries.
`ClickHouse.Driver.Tcp/Types/Converters/ConverterDerivation.Read.cs`	Selects indexer readers.
`ClickHouse.Driver.Tcp/Types/Converters/ColumnReader.cs`	Documents dictionary caching.
`ClickHouse.Driver.Tcp/Poco/PocoValueProjection.cs`	Retains legacy projection path.
`ClickHouse.Driver.Tcp/Poco/PocoTypeRegistry.cs`	Supports forced scatter tiers.
`ClickHouse.Driver.Tcp/Poco/PocoScatterTier.cs`	Defines Emit and Fill tiers.
`ClickHouse.Driver.Tcp/Poco/PocoReadPlan.cs`	Builds converter-based plans.
`ClickHouse.Driver.Tcp/Poco/PocoReadErrors.cs`	Centralizes read diagnostics.
`ClickHouse.Driver.Tcp/Poco/PocoColumnScatterFactory.cs`	Implements converter scatters.
`ClickHouse.Driver.Tcp/Poco/LegacyPocoScatterTier.cs`	Preserves legacy tier names.
`ClickHouse.Driver.Tcp/Poco/LegacyPocoColumnScatterFactory.cs`	Preserves differential reference path.
`ClickHouse.Driver.Tcp.Tests/Types/Converters/ReadRulesTests.cs`	Exposes shared test matrices.
`ClickHouse.Driver.Tcp.Tests/Types/Converters/ReadCombinatorTests.cs`	Compares POCO failure behavior.
`ClickHouse.Driver.Tcp.Tests/Types/Converters/IndexedReaderTests.cs`	Tests indexer-only reads.
`ClickHouse.Driver.Tcp.Tests/Types/Converters/DictionaryEntryCacheTests.cs`	Tests caching and concurrency.
`ClickHouse.Driver.Tcp.Tests/Program.cs`	Adds child-process entry point.
`ClickHouse.Driver.Tcp.Tests/Poco/PocoReadRulesTests.cs`	Tests conversion-rule parity.
`ClickHouse.Driver.Tcp.Tests/Poco/PocoReadRegistration.cs`	Registers differential POCO arms.
`ClickHouse.Driver.Tcp.Tests/Poco/PocoReadPlanTests.cs`	Updates plan coverage.
`ClickHouse.Driver.Tcp.Tests/Poco/PocoReadPlanFillTests.cs`	Repeats plan tests through Fill.
`ClickHouse.Driver.Tcp.Tests/Poco/PocoReadNoDynamicCodeTests.cs`	Tests no-dynamic-code execution.
`ClickHouse.Driver.Tcp.Tests/Poco/PocoColumnScatterFactoryTests.cs`	Tests tier selection and failures.
`ClickHouse.Driver.Tcp.Tests/Poco/LegacyPocoColumnScatterFactoryTests.cs`	Covers legacy tier behavior.
`ClickHouse.Driver.Tcp.Tests/Integration/PocoReadTierIntegrationTests.cs`	Compares tiers against a server.
`ClickHouse.Driver.Tcp.Tests/Integration/PocoReadIntegrationTests.cs`	Uses shared integration fixture.
`ClickHouse.Driver.Tcp.Tests/Integration/PocoReadIntegrationTestBase.cs`	Hosts shared POCO integration tests.
`ClickHouse.Driver.Tcp.Tests/Integration/PocoReadFillIntegrationTests.cs`	Runs integration tests through Fill.
`ClickHouse.Driver.Tcp.Tests/Differential/LegacyPocoRead.cs`	Adds the legacy reference arm.
`ClickHouse.Driver.Tcp.Tests/Differential/ClientArms.cs`	Registers converter-based POCO arms.
`ClickHouse.Driver.Tcp.Tests/ClickHouse.Driver.Tcp.Tests.csproj`	Configures the explicit test entry point.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

alex-clickhouse · 2026-10-11T18:59:08Z

    <!-- codecov.yml ignores the vendored source in ClickHouse.Driver.Common/Vendor, so do not instrument it. -->
    <ExcludeByFile>**/Vendor/**/*.cs</ExcludeByFile>
+    <!-- The assembly has its own entry point (Program.cs), which a test starts in a child process. -->
+    <GenerateProgramFile>false</GenerateProgramFile>


This is not a defect. Microsoft.NET.Test.Sdk sets OutputType to Exe for each .NET target framework, and GenerateProgramFile=false keeps Program.Main as the entry point. dotnet msbuild -getProperty:OutputType gives Exe for net8.0, net9.0 and net10.0, the built dll has an entry point, dotnet exec ClickHouse.Driver.Tcp.Tests.dll --read-poco-rows runs, and PocoReadNoDynamicCodeTests passes on the three frameworks in CI.

polyglotAI-bot · 2026-10-11T15:31:06Z

Summary

This PR moves native TCP POCO reads onto the converter layer, with a compiled loop, a bulk-read fallback, and weakly cached LowCardinality dictionaries. Static review found no actionable issues in the changed behavior or regression coverage. Tests and benchmarks could not be run in this sandbox. ✅ Approve.

Final verdict

✅ Approve

_{Automated review by Polyglot AI (gpt-6-astra) of 38c5aa9. Replies on the inline threads are read on the next review. A maintainer can ask for a new review by mentioning @polyglotAI-bot or requesting a review from it.}

The dictionary readers convert the entries of a column once and keep them in a cache that keys on the column object (a ConditionalWeakTable, safe for concurrent use, which does not keep the column alive). The setup of Emit takes the entries from that cache, and so does the bulk read of a reader that is bound again. So the windows of a POCO read, which run the setup of the compiled loop for each window, and the element reader of an Array or a Map, which that setup binds again, convert the dictionary of a column once. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…e indexer Variant, Dynamic, Nested, QBit, the geo types and Tuple() read only as their canonical type. Their columns make each value when it is read, and their Values makes the values of all the rows into a cache that the column keeps. The reader of these types (IndexedReader) reads the indexer, in Fill and in Emit, so a POCO read of a window of rows makes only the values of that window, as the old POCO scatter did with its indexer tier. The values do not change. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

PocoColumnScatterFactory derives the reader of each POCO column from the column type and the property type (ConverterDerivation, with the read rules of POCO mapping at the root) and runs it in one of two tiers: - Emit: the reader's expression for one row is spliced into one compiled loop for each column, which sets the property of each row. The runtime chooses it where expression trees compile. - Fill: the reader's bulk read into a pooled buffer, then a setter delegate for each row. It compiles no code, so a runtime without dynamic code (RuntimeFeature.IsDynamicCodeCompiled false) uses it. A NULL that a read rule meets throws NullValueException with the row of the column; the scatter throws the POCO message for it, with the row of the result (rowOffset + row - start). The refusal and NULL texts are the ones of the old scatter (PocoReadErrors). Rows are still built with the compiled constructor of the descriptor. The old scatter moves unchanged to LegacyPocoColumnScatterFactory (with LegacyPocoScatterTier), which only the tests call through PocoReadPlan.BuildLegacy. It is the reference of the differential tests (ReferenceArms.Poco, LegacyPocoRead) and of the read-rule matrix test. The client's plan is registered as a candidate for every POCO facet, once with the tier that the runtime chooses and once with Fill forced; no outcome changes. PocoTypeRegistry.ForcedTier lets a test force the tier of a client. Tests changed on purpose: the POCO plan tests read decoded columns where a reader needs the decoded shape of the column type, and name the new tiers; the tests of the old tier selection move to LegacyPocoColumnScatterFactoryTests. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

PocoReadPlanFillTests and PocoReadFillIntegrationTests run the POCO plan tests and the server-backed POCO read tests with PocoScatterTier.Fill forced, the tier of a runtime without dynamic code, which the test host does not choose by itself. PocoReadRulesTests compares each tier of the POCO plan with the old plan over the read-rule matrix of column types and CLR targets (enums, casts, nullable targets, tuples, maps, the array casts), for all the rows and for a window that starts past the first row of the block and of the result. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… type A converter tree reads the decoded shape of its column type, for example the null map and the inner column of a Nullable column. A column that a test builds can have the type name without that shape (a block from the server cannot, and a caller cannot build a Block). The plan then fails when it reads, with a message that names the column and the shape, in both tiers. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The old POCO scatter throws the NULL message with no inner exception. The new scatter kept the reader's NullValueException as the inner exception, whose text names the row of the column, while the message names the row of the result, so the two rows differ after the first window. It now throws the old exception exactly. PocoReadRulesTests compares the inner exception of each failure with the old plan's. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…omments The comments of ColumnReader and of the dictionary readers say that a tree keeps the converted dictionary of each LowCardinality column, in a table that does not keep the column alive. The comment of IndexedReader says that a Tuple() column stores its values. The tier comments say that the compiled loop, not Emit, sets the property, and that a property setter that throws can meet a failure of a later row first in the Fill tier. The comments of the reference path say what it is. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

AssertFailsAsPocoMapping compared the readers with ClientArms.Poco, which is the client's plan on the converter derivation since the POCO tier moved, so the two LowCardinality failure-order tests compared the new path with itself. They compare with ReferenceArms.Poco (the old plan) again, and check that the client's plan, in each tier, gives the failure of the old plan. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The POCO plan tests and the POCO read integration tests run in two fixtures, one for each tier, so a test that names its tiers ran twice. The window test reads in the tier of its fixture. The tests that compare the tiers (Materialize_EveryTier_ProducesTheSameRows, ReadPlanFor_DifferentForcedTiers_CompileTheirOwnPlans and the integration test Materialize_EveryScatterTier_ReadsTheSameRows) move, unchanged, to fixtures that no other fixture derives from. A new test runs the scatter of each tier over the windows of one LowCardinality column and checks that its dictionary is converted once. The two base fixtures carry a justified suppression of NUnit1034. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review round 1 of the POCO reads (ClickHouse/integrations#801), F3: the comment of the span tier in LegacyPocoColumnScatterFactory and the comment of the interpreter test in LegacyPocoColumnScatterFactoryTests keep their text, with a colon and a full stop in place of the dashes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review round 1 of the POCO reads (ClickHouse/integrations#801), F2. A test starts the test assembly in a child process with the runtime switch System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported set to false in its runtime configuration (what the DynamicCodeSupport MSBuild property sets). There RuntimeFeature.IsDynamicCodeCompiled is false, so the POCO read plan chooses the Fill tier by itself, Expression.Compile interprets the compiled constructor of the rows, and Reflection.Emit is not available. The child reads a block of twelve column types (LowCardinality, Nullable, Array, Map, Tuple, Variant, a calendar conversion, an enum ordinal and a cast to object) in windows of two rows, and a NULL into a value type, and its lines must equal those of the compiled loop in the test process. The test project gets its own entry point (GenerateProgramFile is false) for the child; the test runner does not call it. The test takes about 0.3 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…the Cloud job The tests of PocoReadIntegrationTests move, unchanged, to the abstract PocoReadIntegrationTestBase. PocoReadIntegrationTests (the tier that the runtime chooses) carries the Integration and Cloud categories, as before; PocoReadFillIntegrationTests (Fill forced) carries only Integration, because NUnit gives a fixture the categories of its base classes and the Cloud category is for behavior that a Cloud service changes, not for coverage (ClickHouse.Driver.Tcp/AGENTS.md). The names of the tests do not change. Part of ClickHouse/integrations#801. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review round 2 of the POCO reads (ClickHouse/integrations#801), F1. The Fill tier read every row of a window before it called a setter, so a NULL or a failed conversion of a later row came before the exception of a setter at an earlier row, where the compiled loop and the old plan give the setter's exception. The bulk read stays the path of a window that reads without a failure. When it fails, the scatter reads and sets the rows again one at a time and throws the first failure in row order (the failure of the bulk read when no row fails then). A test reads a Nullable(Int32) column of [1, NULL] and of [NULL, 1] into a property whose setter refuses 1, through the old plan, Emit and Fill: the same exception in each. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…e it runs Review round 2 of the POCO reads (ClickHouse/integrations#801), F2. The test read the standard output of the child process to its end before it waited with a timeout, so a child that kept the stream open would hold the test past the timeout. It reads both streams while the process runs, waits for the process with a 2-minute cancellation, kills the process tree when the time passes, and then takes the two streams. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…e#451) * perf(read): pool the response read buffer instead of allocating per query (ClickHouse#670) The reader wrapped the HTTP response in a BufferedStream, which allocates a fresh ReadBufferSize buffer (8 KiB default) per query. Add a small read-only PooledReadBufferStream (ClickHouse.Driver/Formats) that buffers reads over a buffer rented from ArrayPool, and use it on the response path. The BinaryReader -> PeekableStreamWrapper chain does not propagate Dispose to inner streams (PeekableStreamWrapper.Dispose is `new`, not an override), so the reader holds the pooled stream and returns its buffer explicitly on Dispose (idempotent), and on the error path in FromHttpResponseAsync. leaveOpen: true because the HttpResponseMessage owns and disposes the raw content stream. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Raise default ReadBufferSize to 64 KiB now that it's pooled The 8 KiB default (v1.3.0) was chosen because the response read buffer was a fresh per-query allocation and 512 KiB hit the large object heap. Now that the buffer is pooled (rented from ArrayPool, not allocated per query), the small default is no longer necessary. Raise it to 64 KiB to cut read syscalls/refills on large responses; 64 KiB stays under the ~85 KiB LOH threshold, so a pool miss under high concurrency still allocates on the normal heap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address review: fix stale doc default + validate Read arguments - PooledReadBufferStream doc comment said "8 KiB by default"; the default is now 64 KiB. - Read(byte[], offset, count) now validates its arguments per the Stream contract (null array, negative offset/count, offset+count past the buffer) instead of returning 0 or failing later with a less-clear exception. Added a test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: clarify short-read semantics in pooled read buffer comments Drop the LOH/64 KiB rationale from the ReadBufferSize and PooledReadBufferStream doc comments, and document that PooledReadBufferStream.Read may return fewer bytes than requested (callers loop via ExtendedBinaryReader for exact-count reads). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address review: update stale 8 KiB doc + assert against ClickHouseDefaults - ClickHouseClientSettings.ReadBufferSize XML doc said "Default: 8192 (8 KiB)"; update to 64 KiB and note the buffer is now pooled. - ConnectionStringBuilderTests asserted the default against the 8*1024 magic number, which now mismatches; assert against ClickHouseDefaults.ReadBufferSize. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

alex-clickhouse added this pull request to stack #675 October 10, 2026 09:23

This was referenced Oct 10, 2026

TCP converters 6b/6: write through the converter layer everywhere and delete the old write members #677

Open

TCP converters 6c/6: refuse array casts that change the sign of the elements #678

Open

alex-clickhouse marked this pull request as ready for review October 11, 2026 15:14

alex-clickhouse requested a review from mzitnik as a code owner October 11, 2026 15:14

Copilot AI balanced review requested due to automatic review settings October 11, 2026 15:14

Copilot started reviewing on behalf of alex-clickhouse October 11, 2026 15:14

View session

Copilot AI reviewed Oct 11, 2026

View reviewed changes

alex-clickhouse and others added 14 commits October 11, 2026 17:23

alex-clickhouse force-pushed the alex-clickhouse/converter-3-poco-reads branch from 484f6c2 to 38c5aa9 Compare October 11, 2026 18:57

Navigation Menu

Repository navigation

TCP converters 3/6: POCO reads on the fused converter loop - #670

TCP converters 3/6: POCO reads on the fused converter loop#670
alex-clickhouse wants to merge 14 commits into
alex-clickhouse/converter-2b-columnar-readsfrom
alex-clickhouse/converter-3-poco-reads

alex-clickhouse commented Oct 9, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

alex-clickhouse Oct 11, 2026

Uh oh!

polyglotAI-bot commented Oct 11, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alex-clickhouse commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Why

What a user sees

Tests

Benchmarks

Uh oh!

codecov Bot commented Oct 10, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

alex-clickhouse Oct 11, 2026

Choose a reason for hiding this comment

Uh oh!

polyglotAI-bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Final verdict

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alex-clickhouse commented Oct 9, 2026 •

edited

Loading

polyglotAI-bot commented Oct 11, 2026 •

edited

Loading