Add owning-reversible TdsSyncClient over the blocking edge

[Saurabh Singh (saurabh500)]

L4 — Owning-reversible TdsSyncClient over the blocking edge

Stacked on L3 (dev/saurabh/sans-io-expose-l3-blocking-driver @ 7e11a11b) — base is L3, not main.

Surfaces the L3 blocking row driver as a public, reactor-free TdsSyncClient so consumers (ODBC SQLFetch, later py-core) can drop their per-row block_on on the hot path. The async TdsClient surface is unchanged.

Additive-4 public surface (frozen)

• TdsClient::into_sync(self) -> SyncConversion — opportunistic raw-TCP flip.
  • SyncConversion { Converted(TdsSyncClient) | NotEligible(TdsClient) | Failed(TdsError) }. TLS transports return the async client intact via NotEligible (not an error — caller keeps today's block_on).
• TdsSyncClient — owns the established transport + buffer over the blocking edge (concrete StdTcpByteSource; no generic B in the type name). 8 B-free fetch methods (next_row, next_row_into, fetch_rows_batch, take_info_messages, get_metadata, maybe_has_unread_rows, active_plp_reached_end, active_plp_collation).
• TdsSyncClient::into_async(self) -> TdsResult<TdsClient> — reverts the fd to the async client; Err (fd known-dead) on failed revert.

Owning, not borrowed (regression tripwire): a SyncRowFetcher<'a> borrowing dbc.client can't persist across SQLFetch FFI calls, so it would flip the fd per row on a 1M-row set. Owning stores TdsSyncClient by value → fd stays blocking across the whole result set (~2 flips/result-set, measured 13.569µs/pair). Drop does terminal clean-close only; the explicit async revert is into_async — never a Drop-driven fd-revert.

Invariants honored

• Empty public-API diff: the only public diff to tds_client.rs is the additive into_sync; cursor_ops.rs untouched. The shared orchestration added alongside it is pub(crate) (see doc-note 3), so no public signature changes and no generic B leaks into any async signature.
• Verbatim reuse: step_row / drive_row_over_buffer_blocking / plp_collect_step / the L3 blocking driver are called as-is, not reimplemented.
• fetch_rows_batch authored fresh as a thin loop over reactor-free next_row with spare-vec recycling (no async batch twin, no Batch TDS row fetch to make mssql-odbc faster than msodbcsql18 #186 dependency).
• execute* / advance* / close* stay async (reached via into_async). No cold-token blocking driver. AE PLP columns → UnimplementedFeature. read_active_plp_bytes deferred to L5.

Tests

Differential parity vs an all-async oracle (byte-identical), plus the named residual-straddle interleave gate in both directions (async→sync and sync→async where buffered tds_read_buffer bytes straddle the flip mid-packet) and a clean-boundary negative control. The 7-cert lib baseline (4 certificate_validator + 3 win_tls::validate, both fixture/cert-store-gated) is unchanged.

---

Doc-notes (context, not a re-plan)

1. Rule-C WHY = scope + runtime + surface — buffer-size ground was retracted (block_on-over-async-parser inherits the async parser's incremental refill, so buffer-size does not apply). Cold ops (execute/COLMETADATA/advance/close) run once per statement ⇒ zero measurable perf from a sync cold-drive; a block_on cold-token adapter would need a live reactor (multi_thread) and deadlock ODBC's current_thread runtime; fetch-only reuses the async control-plane verbatim. Hence: no cold-token blocking driver.
2. L5 de-risk — the shipped fetch path is reactor-free ⇒ runtime-flavor-agnostic (current_thread is fine). The multi_thread-deadlock concern applies only to the forbidden cold adapter; the earlier prototype multi_thread note was an echo-server co-host artifact (the real SQL peer is remote, so no local task is starved by a blocked thread).
3. Refined-B orchestration decision (drift-proofing) — the sync and async fetch shells share one authoritative token handler rather than parallel copies. The per-token side-effects of the async handle_row_read_token are factored into a single pub(crate) TdsClient::apply_row_read_token(&mut self, token) -> TdsResult<TokenOutcome> (plus pub(crate) finalize_row_error for post-drain cleanup), living on TdsClient in tds_client.rs so it has native access to private state (count_map, finalize_return_value/push_return_value, info buffer) with zero accessors and zero field-visibility churn. Both shells call it; only the ERROR drain is flavour-specific (TokenOutcome::DrainThenError → async drain_stream vs a blocking drain-to-DONE over L3, then the shared finalize_row_error). handle_row_read_token's async signature/visibility is byte-identical — only its body delegates. All connection/result-set state (metadata, INFO buffer, read-till-end flag) lives on the owned inner; the wrapper keeps no mirrored copies, so the two clients cannot diverge. This keeps ClientCore<B> unused (prefer-none) and the public-API diff empty while eliminating the parallel-handler drift risk.
4. Refined-B guard compliance (characterization + error-arm equivalence) —
   • Characterize-then-refactor: 9 characterization unit tests pin each side-effect the shared apply_row_read_token/finalize_row_error owns (terminal DONE accounting + batch close, DONE-MORE accumulation, DONE error-flag → protocol error, ORDER no-op, INFO capture, RETURN_VALUE push, ERROR defer-drain-without-mutation, COLMETADATA usage error, and finalize_row_error batch-clear + surfaced SqlServerError). The full pre-existing async suite passes with zero expected-output edits, so the shared-handler extraction is behavior-preserving.
   • Sync ERROR-arm equivalence: the sync blocking drain-to-DONE now routes every non-terminal side-effect (Info/EnvChange/SessionState/ReturnValue/ReturnStatus) through the same apply_row_read_token, reaching a byte-identical terminal state to the async drain_stream. An Error-token-mid-fetch case was added to both the differential set and the residual-straddle-interleave set (both directions), so the sync drain is exercised, not just the happy path.
   • Visibility reconciliation: because TdsSyncClient lives in a different module (tds_sync_client.rs) and calls inner.apply_row_read_token(...) cross-module, pub(crate) is the tightest reachable visibility (a private fn is unreachable cross-module) — and only new additive pub(crate) methods were introduced. Zero pre-existing item was widened: capture_info_message stays private (the earlier sync path that would have widened it now goes through the shared handler instead).

[Copilot]

Pull request overview

Adds an owning, reversible synchronous row-fetch client over raw TCP while preserving async control-plane operations.

Changes:

• Adds TdsSyncClient and async/blocking transport handoff.
• Shares row-token handling across sync and async paths.
• Adds differential tests and mock mid-stream errors.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
mssql-tds/tests/test_sync_client.rs	Tests sync parity, reversibility, and errors.
mssql-tds/src/io/std_byte_source.rs	Implements blocking TCP reads.
mssql-tds/src/io/packet_buffer.rs	Transfers buffered residual bytes.
mssql-tds/src/io/blocking_reader.rs	Supports seeded buffers and extraction.
mssql-tds/src/io.rs	Registers the blocking source.
mssql-tds/src/datatypes/row_writer.rs	Adds row-buffer recycling.
mssql-tds/src/connection/transport/tds_transport.rs	Defines blocking handoff hooks.
mssql-tds/src/connection/transport/network_transport.rs	Implements raw-TCP extraction/restoration.
mssql-tds/src/connection/tds_sync_client.rs	Implements synchronous fetching and reversal.
mssql-tds/src/connection/tds_client.rs	Adds conversion and shared token handling.
mssql-tds/src/connection.rs	Exports the sync client module.
mssql-mock-tds/src/query_response.rs	Models mid-stream server errors.
mssql-mock-tds/src/protocol.rs	Serializes injected error responses.

Suppressed comments (1)

mssql-tds/src/connection/tds_sync_client.rs:156

• self.active is discarded when the wrapper is consumed, so reverting after next_row_into returned RowPaused or PlpPaused gives the async client an Idle cursor at a mid-row wire position. The PLP case is especially blocking because this sync API deliberately omits read_active_plp_bytes, making into_async the only way to continue. Move the sync pause state back into the inner client's active-row state before returning it.

        let mut inner = self.inner;
        inner
            .transport
            .restore_blocking_parts(tokio_stream, residual)?;

---

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