Skip to content

TCP converters 6a/6: delete the old read members and the reference paths - #674

Open
alex-clickhouse wants to merge 13 commits into
alex-clickhouse/converter-5-poco-writesfrom
alex-clickhouse/converter-6a-delete-read-side
Open

alex-clickhouse wants to merge 13 commits into
alex-clickhouse/converter-5-poco-writesfrom
alex-clickhouse/converter-6a-delete-read-side

Conversation

@alex-clickhouse

@alex-clickhouse alex-clickhouse commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

This is part 6a of the stack of pull requests for ClickHouse/integrations#801, a converter layer for the native TCP client.
It is stacked on #673 (part 5, POCO and untyped writes), which is on #672, #671, #670, #668, #667, #666 and #665. Every read
and write tier of the client goes through the converter derivation since part 5. This part deletes the old read members of
the codecs, the old POCO read and write plans and the old untyped type choice, which stayed only as the reference of the
differential tests, and removes the comparison with the old path from those tests. The codec write surface (the write
shapes, the LowCardinality value and key writers, WritableElementTypes, CanWriteElementType, NullPlaceholderAs)
is the subject of a later part, because some production paths still use it.

What changes for a user

Only the texts of three error messages, which suggest CLR types. They listed the codec's preferred write types, which
miss writes that the converter layer takes, and their composite clause was not exact. Each list now comes from the
derivation, and every type in it is accepted:

  • InsertRowsAsync<T>, a property that the column cannot be written from:
    • before: Column 'code' (FixedString(4)) is filled from property 'Event.Code' of type System.Int32, which it cannot be written from. It accepts System.Byte[], and — for a composite type — rows whose elements are any type its element codecs accept. Give the property one of those types, or insert that column through the columnar API.
    • after: ... It accepts System.Byte[] or System.String, and an Array, Map or Tuple type also accepts rows of the types that its element types accept. Give the property one of those types, or insert that column through the columnar API.
  • InsertRowsAsync with object[] rows, values of a type that the column cannot be written from:
    • before: Column 0 ('value', Array(DateTime('UTC'))) was given values of type System.Guid[], which it cannot be written from. It accepts System.UInt32[].
    • after: ... It accepts System.UInt32[], and an Array, Map or Tuple type also accepts values of the types that its element types accept.
    • For a column that holds NULL, the list names the value type (System.Int32 for Nullable(Int32)), not the nullable
      type that it named before (System.Nullable`1[System.Int32]): a boxed value is never of a nullable type.
  • InsertAsync, a column of an element type that the target cannot be written from:
    • before: Column 'value' (FixedString(4)) was given a column of element type System.Int32, which it cannot be written from. It accepts System.Byte[], and — for a composite type — a column whose elements are any type its element codecs accept.
    • after: ... It accepts System.Byte[] or System.String, and an Array, Map or Tuple type also accepts a column whose elements are of the types that its element types accept.

The lists change for these column types: FixedString(N) (bare, in Nullable, in LowCardinality) adds
System.String; LowCardinality(String) and LowCardinality(Nullable(String)) add System.Byte[];
LowCardinality(DateTime...) and LowCardinality(Nullable(DateTime...)) add the DateTimeOffset and DateTime types.
The read refusals of Block.ReadAs<T> and QueryAsync<T> also take their lists from the derivation, and their texts do not
change: for every column type of the test suites the list is the one of before.

What is deleted

  • The read members of IColumnCodec and of every codec: TryProjectRead, TryProjectColumnRead,
    ReadableElementTypes, with their helpers; LegacyColumnProjection with ProjectedViewCache and the
    ColumnReadProjection delegate; ProjectedReadColumn; the read half of CompositeElementProjections; the expression
    helpers of ColumnValueProjections that only the projections used.
  • The old POCO read plan: LegacyPocoColumnScatterFactory, LegacyPocoScatterTier, PocoValueProjection, PocoSpan,
    PocoReadPlan.BuildLegacy.
  • The old POCO write plan and the old untyped type choice: LegacyPocoColumnBuilderFactory, LegacyPocoWriteConversion,
    PocoWritePlan.BuildLegacy, the UntypedWriteTypeChooser overload; PocoWriteConversion.AcceptedWriteTypes and the
    WritableElementTypes list of NullableColumnCodec, which only the old messages read.
  • In the tests: the copies of the old columnar read and write dispatch and of the old row inserts, and the reference arms.

How

  • ConverterDerivation.SuggestedTypes(type, context, direction) gives the short list of an error message: the CLR types of
    a leaf's pairs in the leaf table, made nullable under Nullable, the same under LowCardinality and
    SimpleAggregateFunction, and the canonical type of any other type. It keeps only the types that the derivation accepts
    for the whole column type. A derivation cannot list what it accepts, so the list leaves out the types of the read and write
    rules and the other types inside a composite.
  • Differential tests. The client's entry point of each tier is now the first candidate of every facet, so it is the
    baseline: the Fill and Emit arms of the converters, the Fill tier of the POCO read plan, the gather with no compiled
    code of the POCO write plan, and ClickHouseTcpTypes.CanWrite in the row tiers are compared with it, for all rows and for a
    tail that starts above row 0. The comparisons with the old path and their declared changes are gone. One family of
    outcomes stays declared: a nullable column read as a value type fails at the first NULL of the column in
    Block.ReadAs, which converts the whole column, and at the first NULL of the tail in a reader of the tail. New tests
    over the reports check, for every case: CanRead agrees with ReadAs and POCO mapping; CanWrite agrees with the insert
    (a type with a Nested in it is written only from a column of its own layout), and the answers of the row tiers agree
    with the row inserts; a decoded column writes the bytes that it was read from, every write of a decoded column (also a
    slice, also through the row inserts) gives the values of the same source rows, and a column read back as another CLR type
    writes the bytes of the source column (except where the CLR calendar types cannot hold the counts of a DateTime64 or
    Time64 of a scale above 7); POCO mapping reads what ReadAs reads; the row inserts write the bytes of the columnar
    insert. The 56
    column types of the read cases are a literal table with the types and targets of before, in place of targets that came
    from ReadableElementTypes; the case list is unchanged (403 cases).

Tests

  • Tests of the deleted members go: the read lists, the expression projections of the codecs, the lifting helper, the view
    cache, the old scatter tiers. Their behavior is covered by the differential tests, ReadRulesTests, ReadCombinatorTests
    and new CanRead cases for the shapes that a composite refuses.
  • Tests that compared with an old plan compare tiers with each other: Fill with Emit, the two POCO read tiers, the two
    POCO gather tiers, the columnar insert with the POCO plan and CanWrite, the untyped insert with CanWrite and with the
    columnar insert of the same values. Failure-order tests state the failing value or text. What the rules accept is pinned
    in two literal tables (the accepted targets of 36 column types for reads, the accepted sources of 51 column types for
    writes), and the read pairs of the leaf table in a literal list for each of 58 sample types.
  • The value and error tests of ColumnReadProjectionTests read each value through Block.ReadAs (renamed from
    TryProjectRead_* to ReadAs_*). The calendar integration tests read through the derived reader.
  • New: SuggestedTypesTests (every leaf, with Nullable, LowCardinality and SimpleAggregateFunction around it, in
    both directions: each suggested type derives, and a leaf suggests all its pairs) and RefusalMessageTests (the exact texts
    of the five refusals that suggest types).
  • net9.0, the full TCP suite with a server: 6391 passed on part 5, 6291 passed here (-100), 0 failed, 0 skipped. Of the
    test cases of part 5, about 260 are gone with the members and comparisons they tested; about 690 have a new name or
    compare with another tier; about 160 are new. net8.0 and net10.0, the offline suite (no server): 3967 passed each.
  • The public API does not change.

Review: two rounds. Round 1 found two items, fixed and pinned by tests: the writes of a decoded column were compared
for all rows only, so a dense write that ignored the start of a slice passed; and the gate FAIL needed the placement
check of the stack (below). Round 2 approved with no findings.

Lines

Diff Production (ClickHouse.Driver.Tcp) Tests Benchmarks Docs Changelog
This part +123 / -2020 +1393 / -2811 0 +4 +13 / -10
The stack from part 0 (77ca7eeb^) +9617 / -1985 +9862 / -2748 +325 / -8 +127 / -18 +31
The stack from main +9622 / -1985 +12642 / -2271 +847 +127 / -18 +31

The write side of the codecs that the later part removes is still in these counts.

Where the speedup of the stack comes from

The converter layer does not convert values faster. Both the old and the new code call the same conversion code
for each value (for example DateTimeToOffset, with the timezone resolved once when the codec is built, and the
same UTF-8 encode and decode). The gains come from work that the old structure did around each value:

  1. Indirection on each row. In the old code, a conversion was a hook on a codec: TryProjectColumnRead gave a
    view, and TryProjectRead gave an expression that was compiled into a delegate for each value. A composite codec
    put the view of its child inside its own view. So one converted value cost a delegate call, an interface cast,
    and interface calls into the column (each of which built a span again), and, for a composite, the same again
    for each element. The POCO scatter used its fast tier only for columns that implement IStoredValuesColumn;
    String, Nullable(...) and LowCardinality(...) columns do not, so each row went through the column's
    indexer. The writes read the caller's column one element at a time through IColumn<T>[i], and Nullable put a
    substitute view around it. The new code derives one tree for the whole type. Bind does the casts and builds
    the spans once for each column (once for each window of 256 rows in a POCO read). The bulk Fill loops are
    generics closed over struct conversions, so the conversion is inlined into the loop. A POCO read splices the
    per-row expression of the whole tree into one compiled loop for each column. For example:
    • LowCardinality(String) read as byte[]: a delegate call, an interface cast, two interface calls and a
      type-checked array store for each row, against two loads and a store.
    • Array(String) read as byte[][]: about 120,000 dispatches for each block of 10,000 rows, against 30,000.
    • Nullable(DateTime) written from DateTimeOffset?: three interface calls for each row, against none.
  2. Allocation and rebuilding for each block. The old ReadAs<T> allocated a new array for the whole converted
    column in each block. At 10,000 rows a DateTimeOffset[] (160 KB) or DateTimeOffset?[] (240 KB) is on the large
    object heap, and a reader that keeps going runs a garbage collection about once in 20 blocks. The new view uses a
    pooled array that the block owns (about 300 bytes allocated for each block). The old LowCardinality writer
    built a new Dictionary with no capacity in each block: for 10,000 distinct values that is 11 resizes, 941 KB
    for each block (719 KB on the large object heap) and a generation 2 collection about once in 10 blocks, and it
    hashed each new value two times. The new interners keep their tables in pooled buffers that grow with the
    number of distinct values, and hash each value one time (1.8 KB for each block for FixedString).
  3. One writer call for each fixed-width value. The old leaf writes made one ClickHouseBinaryWriter call for
    each null byte, each UInt32 and each dictionary key. The new writes put the null map in one copy, convert into
    4 KB chunks on the stack, and write the keys in chunks.

The work that both versions share limits the gain. A read that converts a String leaf only (String as
string, DateTime in a POCO) does the same work for each row in both versions. A LowCardinality dictionary is
converted one time for each block in both. A repeated string into LowCardinality(String) costs one dictionary
lookup for each row in both.

Ratios in the default runtime mode (tiered compilation with dynamic PGO, the steady state), new time / old time
for a block of 10,000 rows, measured with the old and the new code side by side:

Shape new / old
Block.ReadAs: DateTime('UTC') as DateTimeOffset 0.53
Block.ReadAs: Nullable(DateTime('UTC')) as DateTimeOffset? 0.47 to 0.52
Block.ReadAs: LowCardinality(String) as byte[] 0.45
Block.ReadAs: Array(String) as byte[][] 0.76 to 0.79
POCO read: LowCardinality(Nullable(String)) as string 0.80
POCO read: LowCardinality(String) as byte[] 0.72
POCO read: Array(String) as byte[][] 0.79
POCO read: a row of six columns 0.74
POCO read: Nullable(DateTime('UTC')) as DateTimeOffset? 0.87
POCO read: String as string, DateTime('UTC') as DateTimeOffset 0.93, 1.00
InsertAsync: Nullable(DateTime('UTC')) from DateTimeOffset? 0.68 to 0.74
InsertAsync: LowCardinality(FixedString(16)) from byte[], all distinct 0.63 to 0.66
InsertAsync: LowCardinality(String) from string, all distinct 0.90 to 0.93
InsertAsync: LowCardinality(String) from string, 100 distinct 0.97 to 1.05
InsertRowsAsync<T>: Nullable(DateTime('UTC')) from DateTimeOffset? 0.82 to 0.88

The gate in "Benchmarks" below runs with tiered compilation off, so that the code does not change during a run.
Its ratios differ from the ones above: in that mode the JIT cannot remove the old per-row interface calls of
ordinary methods, and it compiles the new loops without the inlining that tier 1 adds.

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 on the same machine. A shape fails when its ratio head/base is above 1.05 in every pair. Measured on
9df449b0; the later commit changes tests only.

base 77ca7eeb, head 9df449b0, 4 rounds (base then head in each), limit 1.05
  round 1: base load average 6.08 to 5.30, CPU busy before 39% and after 12%; head load average 4.96 to 2.79, CPU busy before 11% and after 12%
  round 2: base load average 2.57 to 2.91, CPU busy before 14% and after 11%; head load average 2.67 to 2.86, CPU busy before 11% and after 11%
  round 3: base load average 2.63 to 2.63, CPU busy before 15% and after 13%; head load average 2.42 to 2.51, CPU busy before 12% and after 13%
  round 4: base load average 2.31 to 2.62, CPU busy before 14% and after 12%; head load average 2.41 to 1.70, CPU busy before 13% and after 13%

shape                                                                   base ms  ratio  pairs            spread base/head  verdict
TcpConverterRead.Columnar(ArrayStringAsBytes)                             0.540  0.718  0.730 0.745 0.706 0.701     4.8%/  6.1%  ok
TcpConverterRead.Columnar(DateTimeAsOffset)                               0.264  0.733  0.739 0.736 0.726 0.729     0.9%/  1.7%  ok
TcpConverterRead.Columnar(LowCardinalityNullableStringAsString)           0.031  1.147  1.165 1.132 1.161 1.112     7.6%/  3.0%  FAIL
TcpConverterRead.Columnar(LowCardinalityStringAsBytes)                    0.111  0.325  0.324 0.313 0.325 0.337     0.9%/  8.5%  ok
TcpConverterRead.Columnar(NullableDateTimeAsOffset)                       0.298  0.638  0.623 0.645 0.645 0.630     0.5%/  3.3%  ok
TcpConverterRead.Columnar(StringAsString)                                 0.224  1.014  1.030 1.009 1.012 1.017     0.7%/  1.5%  ok
TcpConverterRead.Columnar(Wide)                                           1.404  0.825  0.814 0.835 0.837 0.812     4.5%/  2.3%  ok
TcpConverterRead.Poco(ArrayStringAsBytes)                                 0.678  0.698  0.683 0.724 0.714 0.658     7.1%/  7.5%  ok
TcpConverterRead.Poco(DateTimeAsOffset)                                   0.279  1.002  0.982 1.019 1.033 0.985     3.0%/  3.2%  ok
TcpConverterRead.Poco(LowCardinalityNullableStringAsString)               0.259  0.468  0.484 0.474 0.463 0.457     4.1%/  9.4%  ok
TcpConverterRead.Poco(LowCardinalityStringAsBytes)                        0.206  0.594  0.605 0.534 0.596 0.592     5.5%/  8.1%  ok
TcpConverterRead.Poco(NullableDateTimeAsOffset)                           0.289  0.889  0.876 0.918 0.903 0.869     2.6%/  3.5%  ok
TcpConverterRead.Poco(StringAsString)                                     0.342  0.932  0.919 0.909 0.968 0.946     4.3%/  4.9%  ok
TcpConverterRead.Poco(Wide)                                               1.557  0.758  0.751 0.750 0.764 0.766     1.9%/  3.4%  ok
TcpConverterWrite.Insert(ArrayStringFromText)                             0.640  0.913  0.943 0.902 0.908 0.917     2.3%/  4.8%  ok
TcpConverterWrite.Insert(LowCardinalityFixedStringBytesHighRepeat)        0.357  1.020  0.993 1.034 1.022 1.019     3.7%/  2.7%  ok
TcpConverterWrite.Insert(LowCardinalityFixedStringBytesLowRepeat)         1.304  0.489  0.478 0.469 0.499 0.502     7.1%/  0.6%  ok
TcpConverterWrite.Insert(LowCardinalityStringHighRepeat)                  0.308  0.960  0.959 0.948 0.965 0.962     1.3%/  0.7%  ok
TcpConverterWrite.Insert(LowCardinalityStringLowRepeat)                   1.249  0.823  0.822 0.835 0.809 0.825     2.9%/  2.6%  ok
TcpConverterWrite.Insert(NullableDateTimeFromOffset)                      0.156  0.397  0.404 0.396 0.395 0.398     0.7%/  2.0%  ok

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

The one FAIL, Columnar(LowCardinalityNullableStringAsString) at 1.15x, 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. Part 3 found the same effect on the same shape: the benchmarks run with tiered compilation off, so the JIT compiles
each method once, at the next free place in its code memory, and the method that reads the column
(NullableLowCardinalityReferenceColumn<T>.Values) is slower when it starts at offset 0 (mod 64) than at offset 32:

  • The disassembly of that method, and of StringColumn.Values, is the same on part 0 and on this part. Only the embedded
    addresses differ.
  • In the benchmark's processes of that shape, the method starts at offset 32 (mod 64) on part 0 and at offset 0 on this part.
    This part deletes code, so the setup compiles other methods before it.
  • In one build of this part, a test program that runs the benchmark's own setup and read: one extra small method compiled
    first moves the method from offset 0 to 32 and the read from 35.0 µs to 30.4 µs a block (median of the minimums of 10 runs);
    two extra methods move it back, and it takes 35.2 µs.
  • With tiered compilation on (the default runtime mode), part 0 and this part interleaved, 10 runs each: 33.5 µs and 33.8 µs a
    block (1.01x).

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

codecov Bot commented Oct 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ypes/Converters/ConverterDerivation.Suggestions.cs 95.83% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

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.

🔵 Needs a closer look

The cross-cutting legacy-path removal and reported benchmark-gate failure warrant final human review despite passing CI and extensive replacement coverage.

0 open findings

What changed in this PR

Removes obsolete native TCP converter reference paths now that all read/write tiers use converter derivation, while preserving coverage through tier parity and literal behavior matrices.

Changes:

  • Deletes legacy codec projections and POCO/reference implementations.
  • Derives diagnostic type suggestions from converter rules.
  • Reworks differential tests around client entry-point baselines and updates docs/changelog.
File Description
ClickHouse.Driver.Tcp.Tests/​Differential/​Arms.cs Simplifies baseline-arm wrappers.
ClickHouse.Driver.Tcp.Tests/​Differential/​ClientArms.cs Makes client entry points baselines.
ClickHouse.Driver.Tcp.Tests/​Differential/​DifferentialCase.cs Updates baseline terminology.
ClickHouse.Driver.Tcp.Tests/​Differential/​DifferentialCases.cs Pins literal read targets.
ClickHouse.Driver.Tcp.Tests/​Differential/​DifferentialEngine.cs Compares candidates with baselines.
ClickHouse.Driver.Tcp.Tests/​Differential/​DifferentialRegistry.cs Registers client baselines and outcomes.
ClickHouse.Driver.Tcp.Tests/​Differential/​DifferentialSelfTests.cs Tests revised differential semantics.
ClickHouse.Driver.Tcp.Tests/​Differential/​DifferentialTests.cs Adds cross-tier consistency checks.
ClickHouse.Driver.Tcp.Tests/​Differential/​Expectation.cs Updates declared-outcome support.
ClickHouse.Driver.Tcp.Tests/​Differential/​LegacyColumnarRead.cs Deletes legacy read reference.
ClickHouse.Driver.Tcp.Tests/​Differential/​LegacyColumnarWrite.cs Deletes legacy write reference.
ClickHouse.Driver.Tcp.Tests/​Differential/​LegacyPocoRead.cs Deletes legacy POCO-read reference.
ClickHouse.Driver.Tcp.Tests/​Differential/​LegacyRowWrite.cs Deletes legacy row-write reference.
ClickHouse.Driver.Tcp.Tests/​Differential/​Outcome.cs Updates baseline terminology.
ClickHouse.Driver.Tcp.Tests/​Differential/​RowWriteArms.cs Simplifies client row-write arms.
ClickHouse.Driver.Tcp.Tests/​Integration/​ColumnReadProjectionIntegrationTests.cs Tests derived calendar readers.
ClickHouse.Driver.Tcp.Tests/​Poco/​LegacyPocoColumnScatterFactoryTests.cs Removes legacy scatter tests.
ClickHouse.Driver.Tcp.Tests/​Poco/​PocoColumnBuilderFactoryTests.cs Compares gather tiers directly.
ClickHouse.Driver.Tcp.Tests/​Poco/​PocoColumnScatterFactoryTests.cs Removes legacy-plan comparisons.
ClickHouse.Driver.Tcp.Tests/​Poco/​PocoReadRegistration.cs Uses client POCO baseline.
ClickHouse.Driver.Tcp.Tests/​Poco/​PocoReadRulesTests.cs Compares Fill and Emit tiers.
ClickHouse.Driver.Tcp.Tests/​Poco/​PocoWritePlanTests.cs Compares row and columnar writes.
ClickHouse.Driver.Tcp.Tests/​Poco/​RowWriteRegistration.cs Removes legacy row registrations.
ClickHouse.Driver.Tcp.Tests/​Types/​ColumnReadProjectionTests.cs Exercises values through ReadAs.
ClickHouse.Driver.Tcp.Tests/​Types/​CompositeElementProjectionTests.cs Removes obsolete projection tests.
ClickHouse.Driver.Tcp.Tests/​Types/​CompositeLiftMatrixTests.cs Uses public read capability checks.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​ConverterHarness.cs Replaces old reads with ReadAs.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​LeafConverterRegistration.cs Removes legacy change declarations.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​LeafConverterRegistrationTests.cs Verifies client-baseline coverage.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​LeafReaderTests.cs Compares readers with ReadAs.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​LeafSamples.cs Updates reference terminology.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​LeafTableTests.cs Removes obsolete codec-list checks.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​ReadCombinatorTests.cs Updates reader parity coverage.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​ReadConverterRegistration.cs Declares nullable-tail outcomes.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​ReadRulesTests.cs Pins accepted read targets.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​RefusalMessageTests.cs Pins five diagnostic messages.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​SuggestedTypesTests.cs Tests derived suggestions.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​WriteConverterRegistration.cs Removes old-path comparisons.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​WriteRulesRegistration.cs Deletes obsolete declarations.
ClickHouse.Driver.Tcp.Tests/​Types/​Converters/​WriteRulesTests.cs Pins accepted write sources.
ClickHouse.Driver.Tcp.Tests/​Types/​FixedStringColumnCodecTests.cs Tests derived text reading.
ClickHouse.Driver.Tcp.Tests/​Types/​NullableColumnCodecTests.cs Tests nullable write acceptance.
ClickHouse.Driver.Tcp/​Poco/​LegacyPocoColumnBuilderFactory.cs Deletes legacy POCO builder.
ClickHouse.Driver.Tcp/​Poco/​LegacyPocoColumnScatterFactory.cs Deletes legacy POCO scatter.
ClickHouse.Driver.Tcp/​Poco/​LegacyPocoScatterTier.cs Deletes legacy tier enum.
ClickHouse.Driver.Tcp/​Poco/​LegacyPocoWriteConversion.cs Deletes legacy conversion rules.
ClickHouse.Driver.Tcp/​Poco/​PocoColumnBuilder.cs Uses derived write suggestions.
ClickHouse.Driver.Tcp/​Poco/​PocoColumnScatterFactory.cs Uses derived read suggestions.
ClickHouse.Driver.Tcp/​Poco/​PocoReadErrors.cs Accepts derived suggestion lists.
ClickHouse.Driver.Tcp/​Poco/​PocoReadPlan.cs Removes legacy plan construction.
ClickHouse.Driver.Tcp/​Poco/​PocoSpan.cs Deletes obsolete span helper.
ClickHouse.Driver.Tcp/​Poco/​PocoValueProjection.cs Deletes legacy value projections.
ClickHouse.Driver.Tcp/​Poco/​PocoWriteConversion.cs Removes preferred-type enumeration.
ClickHouse.Driver.Tcp/​Poco/​PocoWriteErrors.cs Improves derived-type diagnostics.
ClickHouse.Driver.Tcp/​Poco/​PocoWritePlan.cs Removes legacy write plan.
ClickHouse.Driver.Tcp/​Poco/​UntypedRowColumns.cs Removes legacy type chooser.
ClickHouse.Driver.Tcp/​Protocol/​ClickHouseTcpConnection.cs Uses derived insert suggestions.
ClickHouse.Driver.Tcp/​Types/​Codecs/​ArrayColumnCodec.cs Removes read projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​DateTime64ColumnCodec.cs Removes read projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​DateTimeColumnCodec.cs Removes read projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​EnumColumnCodec.cs Removes label projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​FixedStringColumnCodec.cs Removes text projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​LowCardinalityColumnCodec.cs Removes dictionary projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​MapColumnCodec.cs Removes map projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​NullableColumnCodec.cs Removes nullable projections/lists.
ClickHouse.Driver.Tcp/​Types/​Codecs/​StringColumnCodec.cs Removes byte projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​Time64ColumnCodec.cs Removes read projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​TimeColumnCodec.cs Removes read projections.
ClickHouse.Driver.Tcp/​Types/​Codecs/​TupleColumnCodec.cs Removes tuple projections.
ClickHouse.Driver.Tcp/​Types/​ColumnReadProjections.cs Uses derived read suggestions.
ClickHouse.Driver.Tcp/​Types/​ColumnValueProjections.cs Removes obsolete expression helpers.
ClickHouse.Driver.Tcp/​Types/​CompositeElementProjections.cs Retains write-side array helper.
ClickHouse.Driver.Tcp/​Types/​Converters/​ConverterDerivation.Suggestions.cs Implements diagnostic suggestions.
ClickHouse.Driver.Tcp/​Types/​Converters/​LeafReaders.cs Updates wire-byte diagnostic reference.
ClickHouse.Driver.Tcp/​Types/​IColumnCodec.cs Removes legacy read members.
ClickHouse.Driver.Tcp/​Types/​LegacyColumnProjection.cs Deletes legacy projection infrastructure.
ClickHouse.Driver.Tcp/​Types/​ProjectedReadColumn.cs Deletes legacy projected view.
changelog.d/​converter-2b-columnar-reads.improvements.md Consolidates prior stack wording.
changelog.d/​converter-4b-columnar-writes.improvements.md Consolidates insert scope.
changelog.d/​converter-4b-lowcardinality-bytes.fixes.md Clarifies all-insert coverage.
changelog.d/​converter-5-write-rules.improvements.md Consolidates write-rule wording.
changelog.d/​converter-6a-refusal-messages.improvements.md Records diagnostic changes.
docs/​tcp.mdx Documents shortened suggestion lists.

🧠 Review effort: Balanced


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

@polyglotAI-bot

polyglotAI-bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary
This PR removes obsolete conversion paths from the native TCP client, derives error-message type suggestions from the active converters, and replaces legacy comparisons with fixed expectations and comparisons between current implementations. No actionable findings were identified in this snapshot. ✅ Approve. Builds and tests were not run in this sandbox.

Final verdict

✅ Approve

Automated review by Polyglot AI (gpt-6-astra) of 9603d21. 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.

… tests

The old path of every tier is now only a reference for tests that the next
commits remove, so the differential fixture stops comparing with it
(ClickHouse/integrations#801).

- The client's entry point of each tier is the first candidate of every
  facet, so it is the baseline. The converter arms (Fill and Emit), the
  Fill tier of the POCO read plan, the delegate gather of the POCO write
  plan and ClickHouseTcpTypes.CanWrite in the row tiers are compared with
  it. The reference arms and the declared changes against the old path go.
- One family of outcomes stays declared: a nullable column read as a type
  that cannot hold NULL fails at the first NULL of the column in
  Block.ReadAs, and at the first NULL of the tail in a reader of the tail.
- The column types of the read-projection cases are a literal table with
  the 56 types and targets of before, in place of the targets that came
  from ReadableElementTypes and the test attributes.
- New cross-tier tests: CanRead agrees with ReadAs and POCO mapping,
  CanWrite with the insert, a decoded column writes the bytes it was read
  from, POCO mapping reads what ReadAs reads, and the row inserts write the
  bytes of the columnar insert.
- The baseline outcome counts replace the reference outcome counts.
The POCO read plan has read through the converter derivation since part 3
of ClickHouse/integrations#801. The old scatter stayed only as the
reference of the tests.

Deleted: LegacyPocoColumnScatterFactory, LegacyPocoScatterTier,
PocoValueProjection with PocoProjectionSite, PocoSpan and
PocoReadPlan.BuildLegacy, and the test arm and tests of the old scatter.

The tests that compared with the old plan compare without it:
PocoReadRulesTests runs the Fill tier against the Emit tier over the read-rule
matrix; the read-rule matrix of ReadRulesTests compares Emit and the client's POCO
plan with Fill; the two dictionary failure-order tests state the failure
text; the setter test runs the two tiers.
A derivation cannot list what it accepts, so the read refusals of
Block.ReadAs and of the POCO read plan need a short list of CLR types to
suggest. ConverterDerivation.SuggestedTypes gives it: the CLR types of a
leaf's pairs in the leaf table, made nullable under Nullable, the same
under LowCardinality and SimpleAggregateFunction, and the canonical type of
any other type. It keeps only the types that the derivation accepts for the
whole column type. ClickHouse/integrations#801.

For every column type of the test suites the read lists are the ones that
ReadableElementTypes gave, so the texts of the read refusals do not change.
A test checks, for every leaf and for Nullable, LowCardinality and
SimpleAggregateFunction around it, that each suggested type derives and that
the leaf suggests every CLR type of its pairs.
Block.ReadAs, CanRead and the POCO read plan read through the converter
derivation, and the read refusals take their suggested types from it, so
the column projections of the codecs have no caller
(ClickHouse/integrations#801).

Deleted: IColumnCodec.TryProjectRead, TryProjectColumnRead and
ReadableElementTypes with every codec's implementation and helpers,
LegacyColumnProjection with the ColumnReadProjection delegate and
ProjectedViewCache, ProjectedReadColumn, the read half of
CompositeElementProjections, ColumnValueProjections.RequireSourceType and
TryLiftOverAbsent, StringColumnCodec.RowBytes, FixedStringColumnCodec.RowText
and the test copy of the old columnar dispatch.

Tests: the value and error scenarios of ColumnReadProjectionTests read each
value through Block.ReadAs (the differential tests read the same scenarios);
the tests of the projection API and of the read lists go. The leaf, lift
matrix and combinator tests compare with Block.ReadAs, CanRead, literal
values or the expected failing value. The calendar integration tests read
through the derived reader. The edge cases of the composite projections are
CanRead cases.
The row inserts have written through the converter derivation since part 5
of ClickHouse/integrations#801; the old plan stayed only as the reference of
the tests.

Deleted: LegacyPocoColumnBuilderFactory, LegacyPocoWriteConversion with
PocoGatherSite, PocoWritePlan.BuildLegacy, the UntypedWriteTypeChooser
overload of UntypedRowColumns.CreateSource, and the test copy of the old
row inserts.

Tests: the write rule matrix compares the two gather tiers, the columnar
insert with the POCO write plan and CanWrite, and the untyped insert with
CanWrite, in place of the old plan; an untyped column written through a
cast gives the bytes of the columnar insert of the cast values; the plan
tests that compared with the old plan state the bytes or compare with the
columnar insert of the same values; the getter-and-NULL test states the
first failure and compares the two tiers. The template arms of the row
inserts in the differential tests are one client arm each.
The refusals of the POCO write plan, the untyped rows and the columnar insert
listed the codec's preferred write types, which miss writes that the
converter derivation takes: "It accepts System.Byte[]" for FixedString(4),
which is also written from string, and no byte[] for LowCardinality(String)
(ClickHouse/integrations#792). They now take the list from
ConverterDerivation.SuggestedTypes, and the composite clause names what is
true: an Array, Map or Tuple type also accepts the types that its element
types accept. PocoWriteConversion.AcceptedWriteTypes goes. ClickHouse/integrations#801.

Tests: the exact texts of the five refusals that suggest types (columnar and
POCO reads, which do not change, and the three writes).
…exts

The fragments of the converter parts describe the final behavior in the
past tense: the read rules and the whole-column view of Block.ReadAs, the
writes of the converter layer in every insert, the one set of write rules,
and ClickHouse/integrations#792 in every insert. A new fragment covers the
write refusals that name true CLR types. docs/tcp.mdx says what such a
message lists. ClickHouse/integrations#801.
A boxed value is never a nullable value type, so for a column that holds
NULL the refusal of the untyped insert named a type that no value has
(System.Nullable`1[System.Int32]). It names the value type
(System.Int32), which the insert takes. ClickHouse/integrations#801.
MapColumnCodec.Row read a row of the old Map projection, and the
WritableElementTypes list of NullableColumnCodec served only the old
refusal messages (its own CanWriteElementType asks the inner codec). The
Nullable test of that list checks CanWriteElementType for the same types.
Also comment and message wording that named the old projection or path,
and two test parameters that no caller passes.
ClickHouse/integrations#801.
With no old plan to compare with, the read-rule matrices compared the tiers only
with each other, so a rule that started or stopped accepting a type would
pass. Each matrix now pins the accepted types of every column type in a
literal table, and LeafTableTests pins the read pairs of every sample type.

The differential tests also check that the answers of the row tiers are
whether the row inserts accept the column, that a column read back as
another CLR type writes the bytes of the source column (except where the
CLR type cannot hold the counts of a DateTime64 or Time64 of a scale above
7), and that the first candidate of each facet is the client's arm itself.
ClickHouse/integrations#801.
The write of a decoded column (the columnar insert writes it from its
storage) was compared with the source bytes for all rows only, and it is
the only candidate of its facet, so a dense write that ignored the start of
a slice passed. The row inserts of the decoded column had no comparison at
all. Every write of the decoded column, in the three write tiers, for all
rows and for the tail, now decodes to the values of the same rows of the
source column (equal bytes need no decode; a slice of a decoded
LowCardinality column keeps the dictionary of the whole column, so its
bytes can differ). Self-tests: a dense write that ignores the start of
Array(UInt8) is reported, the client's write is not. A dense Array write
that ignores the element base of the slice fails the check on every
decoded Array case. ClickHouse/integrations#801 (review round 1, F2).
@alex-clickhouse
alex-clickhouse force-pushed the alex-clickhouse/converter-6a-delete-read-side branch from 9a9e654 to 9603d21 Compare October 11, 2026 18:57

This branch has not been deployed

No deployments
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