Sans-I/O TDS core (4/N): invert the non-PLP column decode to a column-atomic step()

[Saurabh Singh (saurabh500)]

⚠️ Perf-neutral layer. Part 4 of the green-at-every-step sans-I/O stack. Base: dev/saurabh/sans-io-l3-token-decoder (#194). Public API is unchanged; mssql-odbc and mssql-py-core build unchanged.

Description

Inverts the entire non-PLP column decode path into a column-atomic, re-drivable synchronous step(), moving the .await refill point out of the middle of a column to the column boundary. This removes the last class of mid-value suspension in the row decode path (the risk L2's NeedBytes + L3's sync token decode were built to eliminate) without introducing any new resumable state machine — RowPauseState.next_column_index remains the only resume granularity and NeedBytes { shortfall } the only suspension signal.

What this lands (6 commits, green at each)

• edc7aa83 — invert non-PLP fixed-width column decode to a column-atomic step() over column_wire_len (peek) + whole-cell ensure + decode_column_body (infallible take_*).
• 50dae22d — extend the step to variable-length non-PLP cells (char/varchar/nchar/nvarchar non-max, binary/varbinary non-max, decimal/numeric).
• 952db191 — extend the step to SQL_VARIANT cells.
• 0d5e9619 — converge the async row decoder onto the single shared decode_column_body so no non-PLP leaf logic is duplicated between decoder.rs and sync_decoder.rs.
• ca80c4ab — fold the Always-Encrypted non-PLP ciphertext whole-cell buffer into the sync step.
• 1d46dd27 — mandatory refill-boundary tests (below).

Design

decode_or_decrypt_column owns buffering for all non-PLP cells, so it is inverted as one unit — there is no fixed-width-only seam inside it, and a partial inversion would fork it into a half-sync/half-async mid-column path. Per column the driver runs: column_wire_len (peek-only length) → ensure(total) (whole cell) → decode_column_body (pure sync, exactly one write_*). Because steps 1–2 only peek, a NeedBytes re-drive after refill re-runs from the column start with nothing consumed and nothing written → atomic.

decode_column_body (+ column_wire_len + the sync leaf ports) has exactly one definition in sync_decoder.rs; two refill drivers (sync PacketReader, async GenericDecoder::decode_into) wrap it and differ only by the .await on refill.

Out of scope (unchanged)

• PLP/LOB streaming (PlpColumnStream / PlpChunkStreamReader, incl. AE over varbinary(max)) — L4b, behind the existing pause branch.
• NBCROW body — L4c; the sync step is written so it can be reused verbatim there.

Tests

Five mandatory boundary tests drive the buffer-owning test PacketReader, so they genuinely exercise decode_column_body:

1. non-PLP → PLP transition at every refill boundary (nonplp_to_plp_transition_is_byte_identical_across_refill_boundary) — a [int4][nvarchar(max) PLP] row swept with the packet boundary at each interior offset; the inverted step fully consumes the int4, then the PLP column resumes byte-identically across every split.
2. mid-cell re-drive (var_length_cell_redrive_is_byte_identical_across_refill_boundary) of an inverted var-length cell, including inside the 2-byte length prefix (peek-only re-drive) and mid-data.
3. hybrid mid-row seam (mixed_row_inverted_then_plp_is_byte_identical_across_refill_boundary) — a [int4][varchar][nvarchar(max) PLP] row that mixes two inverted non-PLP cells with a not-yet-inverted (legacy async) PLP cell in ONE row, swept across every offset (int4→varchar seam, inside the varchar length prefix, and the varchar→PLP handoff). Proves the inverted step and the async PLP path share one coherent row cursor (next_column_index) regardless of where the packet boundary lands — the L4a→L4b hybrid-window correctness guarantee.
4. exact whole-column non-PLP → PLP transition (refill_boundary_at_nonplp_to_plp_column_transition_resumes_into_async_plp) — same [int4][varchar][nvarchar(max) PLP] row, but the packet boundary is forced to land exactly at the varchar→PLP column transition: the inverted step fully consumes the varchar cell, ensure on the PLP column's first bytes returns NeedBytes, refill happens, and the still-async PLP path resumes at next_column_index. Asserts byte-identical to the single-packet baseline — the precise whole-column seam the hybrid window depends on.
5. inverted var-length cell own re-drive (refill_boundary_within_inverted_varchar_cell_resumes_coherently) — the boundary is swept inside the inverted varchar cell (within its 2-byte length prefix and mid-data), proving the inverted step's own re-drive from the column start is coherent independent of the PLP handoff.

Making these meaningful required fixing the test PacketReader to be faithful to the production transport: receive_packet now validates the header length, returns the single packet's length, and carries surplus bytes forward via record_pending (previously it returned the raw coalesced byte count and stripped only the first header). TestPacketBuilder now writes the TDS total length (header + payload) into the header field, matching the production serializer and validate_packet_length.

Related Issues

N/A — internal sans-I/O refactor, no tracking issue (consistent with #189/#191/#194).

Checklist

• cargo bfmt passes
• cargo bclippy passes
• cargo btest passes (lib green; failing-name set byte-for-byte identical to base = 359 pre-existing env/cert failures)
• New/changed functionality has tests
• Public API changes are documented (no public API change)

[Copilot]

Pull request overview

Moves non-PLP TDS column decoding into a shared synchronous, column-atomic core while retaining async refill drivers.

Changes:

• Adds synchronous decoding for fixed, variable-length, encrypted, and sql_variant cells.
• Routes test and production packet readers through whole-cell decoding.
• Adds refill-boundary and buffer atomicity tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
mssql-tds/src/datatypes.rs	Registers the synchronous decoder module.
mssql-tds/src/datatypes/decoder.rs	Shares decoding and decryption logic.
mssql-tds/src/datatypes/sync_decoder.rs	Implements column-atomic decoding.
mssql-tds/src/io/token_stream.rs	Routes row columns through the new path.
mssql-tds/src/io/packet_reader.rs	Adds the column-decoding driver and boundary tests.
mssql-tds/src/io/packet_buffer.rs	Adds non-consuming peeks and staged buffers.
mssql-tds/src/connection/transport/network_transport.rs	Drives synchronous decoding over production buffering.

Suppressed comments (1)

mssql-tds/src/datatypes/sync_decoder.rs:302

• The body decoder also reads legacy Char/VarChar with a two-byte prefix, while these wire types use a one-byte BYTELEN prefix (sqldatatypes.rs:467-470). Split them from the modern Big*/Unicode arm and decode their one-byte framing with the appropriate BYTELEN null semantics; otherwise every valid value consumes the wrong bytes.

        TdsDataType::Char
        | TdsDataType::VarChar
        | TdsDataType::BigChar
        | TdsDataType::BigVarChar
        | TdsDataType::NChar
        | TdsDataType::NVarChar => {

---

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

[Saurabh Singh (saurabh500)]

Closing: the sans-I/O restructuring is not going to be productionized.

The row-decode performance work that motivated much of this has been re-scoped around #247, where benchmarked spikes show the dominant win comes from a far smaller change — converting TdsPacketReader to RPITIT to remove per-read boxing (#252, measured at −61.6% decode time) — rather than from inverting the core to a sync step() driver. Given that, this stack is a large amount of surface area for a win that is already available more cheaply.

This is cleanup rather than a rejection of the analysis. The branch is deliberately not deleted, so the work remains recoverable if the direction is revisited.