Sans-I/O TDS core (4c/N): invert the NBCROW null-bitmap read to sync

[Saurabh Singh (saurabh500)]

Summary

Sans-I/O TDS refactor stack L4c (part of stack #192, off #186). Finishes the row-decode inversion by making the NBCROW path fully sync.

After L4a (non-PLP columns sync) and L4b (PLP collect sync), the only NBCROW-specific async wire read left was the fixed-width null-bitmap read at row entry. This inverts exactly that byte pull to the shared sync PacketBuffer core.

What changed

• PacketBuffer::take_bytes(n) — atomic owned take, mirrors the scalar take_* accessors.
• TdsPacketReader::read_null_bitmap(bitmap_len) — new seam modeled 1:1 on decode_column_into:
  • default: assemble via read_bytes (non-buffer-owning readers),
  • PacketReader override: ensure(n) + take_bytes (atomic),
  • NetworkTransport override: ensure_or_refill(n) + take_bytes (atomic),
  • Box<dyn> blanket forward.
• receive_row_into_internal NBCROW arm routes through read_null_bitmap.

Async and sync now share one bitmap-read body. The whole NBCROW eager row is sync by construction: bitmap (L4c) + non-PLP columns (L4a) + PLP collect (L4b).

No new machines

Zero new resumable machines. Reuses PacketBuffer::ensure/NeedBytes{shortfall} and the existing RowPauseState.nbc_null_bitmap (bitmap read once at entry, carried across pauses, never re-read). ensure keeps the forward-progress debug_assert; the take is all-or-nothing so a short buffer re-drives from row entry with nothing partial copied. Byte-identical async output.

Tests (buffer-driven PacketReader + two_packets split-sweep)

• nbcrow_bitmap_read_resumes_byte_identical_across_refill_boundary — 9 columns force a 2-byte bitmap; boundary swept INSIDE the multi-byte bitmap; byte-identical to single-packet baseline.
• fully_sync_nbcrow_mixed_row_is_byte_identical_across_refill_boundary — NBCROW row mixing NULL-via-bitmap, non-PLP inverted (int4 + varchar(64)), and PLP (varbinary(max)); boundary swept across bitmap end, column transitions, inside the PLP chunk.
• Existing nbcrow_pause_and_plp_resume_path_is_exercised stays green.

Validation

• git merge-base --is-ancestor 149fa6f9 HEAD: exit 0.
• cargo nextest -p mssql-tds --lib --no-fail-fast: FAIL-set == exactly the 7 known cert fixtures (Compare-Object vs baseline EMPTY); no hang.
• 3 blocking tests pass by name; cargo bfmt clean; cargo bclippy (-D warnings) clean; scripts/bfmt.ps1 + scripts/bclippy.ps1 clean; mssql-py-core builds.
• LOC delta: +278 / -2 (production +57; rest tests).

Draft — do not merge/mark ready. Base = dev/saurabh/sans-io-l4b-variable-plp. Owned by the L4 stack coordinator (stack #192).

[Copilot]

Pull request overview

Moves NBCROW null-bitmap reads onto the shared synchronous packet-buffer path.

Changes:

• Adds atomic PacketBuffer::take_bytes.
• Adds and forwards read_null_bitmap.
• Adds packet-boundary regression tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
token_stream.rs	Routes NBCROW decoding through the new seam and adds tests.
packet_reader.rs	Defines and implements read_null_bitmap.
packet_buffer.rs	Adds atomic owned byte extraction.
network_transport.rs	Implements buffered bitmap reads for production transport.

Suppressed comments (1)

mssql-tds/src/io/token_stream.rs:2158

• This repeats the full decode and packet-splitting harness from the test immediately above. Reuse a shared module-level helper and leave each test responsible only for its metadata, payload, and assertions; otherwise fixes to the boundary simulation must be kept synchronized manually.

        async fn decode(read_data: Vec<u8>, columns: &[ColumnMetadata]) -> Vec<ColumnValues> {
            let mut mock = MockNetworkReaderWriter::new(read_data, 0);
            let mut reader = PacketReader::new(&mut mock);
            reader.read_tds_packet_for_test().await.unwrap();

---

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