Skip to content

Rewire mssql-odbc fetch hot path onto the reactor-free sync core (L5) - #204

Draft
Saurabh Singh (saurabh500) wants to merge 1 commit into
dev/saurabh/sans-io-expose-l4-tdssyncclientfrom
dev/saurabh/rewire-odbc-sync-core-l5
Draft

Rewire mssql-odbc fetch hot path onto the reactor-free sync core (L5)#204
Saurabh Singh (saurabh500) wants to merge 1 commit into
dev/saurabh/sans-io-expose-l4-tdssyncclientfrom
dev/saurabh/rewire-odbc-sync-core-l5

Conversation

@saurabh500

@saurabh500 Saurabh Singh (saurabh500) commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

L5 — Rewire mssql-odbc fetch onto TdsSyncClient

Stacked on the frozen L4 tip (dev/saurabh/sans-io-expose-l4-tdssyncclient @ 36bc4648). Append-only; base = L4 branch, not main.

What this does

Plaintext raw-TCP connections now serve SQLFetch through the reactor-free TdsSyncClient (blocking std reads, no tokio reactor). TLS and other non-raw transports fall back to the untouched async block_on path, byte-identically — the speedup is opportunistic on plaintext with zero user-visible flag.

Ownership machine

  • DbcClient { Async(TdsClient) | Sync(TdsSyncClient) } in the connection state.
  • Flip to the sync edge once per row-returning result set at execute time (finish_execute), persisting across all SQLFetch calls.
  • Revert to async (into_async) at SQLMoreResults / close / free before driving the existing advance / close_query.
  • Runtime-context hard rule: every into_sync() runs inside dbc.runtime.enter() so the runtime handle is captured for the later revert (a naked into_sync() on the bare ODBC thread would capture None and poison into_async). Implemented in one place: flip_to_fetch_edge.

Batch buffer (batch-ready, per-row default)

  • Statement-level row buffer + spare-row recycling pool, gated behind SYNC_FETCH_DEFAULT_MAX_ROWS (default 1 = per-row byte-identical INFO/SWI parity; flip to 64 is a one-line change).
  • Deferred mid-batch error handling: at max_rows > 1, buffered rows are served before the error surfaces, reproducing the async row-then-error ordering.

Other

  • Cache server_version at connect so SQL_DBMS_VER no longer reads the live client while a sync cursor is open.
  • C-ABI unchanged: no #[unsafe(no_mangle)] extern "C" signatures touched.

Test-infra note (flagged for scope)

mssql-mock-tds (test-only crate, not frozen L4) is extended additively for multi-result-set responses — needed to drive the SQLMoreResults interleave test over a real TCP peer. Single-set responses serialize byte-identically; its own 16 tests stay green.

Tests (in-crate; the driver is a cdylib, so an external tests/ crate can't link it or reach pub(crate) entry points)

  • (a) differential: sync SQLFetch @ max_rows=1 == async oracle, byte-identical
  • (b) max_rows=64: identical row-set + surfaced error (deferred-error fix)
  • (c) SQLMoreResults interleave: sync-fetch → into_asyncadvance → re-into_sync, both sets verified
  • (d) NotEligible fallback: flip_to_fetch_edge keeps the DBC on the async edge

Gates: cargo bfmt clean, cargo bclippy (-D warnings) clean, cargo nextest -p mssql-odbc = 472 passed (468 existing + 4 new), mssql-mock-tds 16 passed.

Part of the sans-I/O native stack - see cover PR #189 for the stack-level summary and tracked debt.

@saurabh500
Saurabh Singh (saurabh500) force-pushed the dev/saurabh/rewire-odbc-sync-core-l5 branch from 87e2e72 to 5c09f0e Compare August 10, 2026 03:09
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as ready for review August 10, 2026 03:21
@saurabh500
Saurabh Singh (saurabh500) requested a review from a team as a code owner August 10, 2026 03:21
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as draft August 10, 2026 03:26
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as ready for review August 10, 2026 06:38
Copilot AI balanced review requested due to automatic review settings August 10, 2026 06:38

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.

Pull request overview

Reworks plaintext ODBC fetching to use the reactor-free TdsSyncClient, while retaining async fallback for unsupported transports.

Changes:

  • Adds async/sync client ownership transitions around execution, fetching, and result boundaries.
  • Adds batched row buffering and sync-fetch integration tests.
  • Extends the mock TDS server with multi-result-set responses.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
mssql-odbc/Cargo.toml Adds mock-server test dependency.
mssql-odbc/src/handles/stmt.rs Adds fetch buffering state.
mssql-odbc/src/handles/dbc.rs Adds dual client ownership and version cache.
mssql-odbc/src/api/sync_fetch_tests.rs Tests sync fetching and transitions.
mssql-odbc/src/api/prepare.rs Resets buffered fetch state.
mssql-odbc/src/api/more_results.rs Transitions clients across result sets.
mssql-odbc/src/api/mod.rs Registers sync-fetch tests.
mssql-odbc/src/api/get_type_info.rs Resets buffered fetch state.
mssql-odbc/src/api/get_info.rs Reads cached server version.
mssql-odbc/src/api/fetch.rs Implements sync and buffered fetching.
mssql-odbc/src/api/execute.rs Resets fetch state before execution.
mssql-odbc/src/api/exec_direct.rs Resets fetch state before direct execution.
mssql-odbc/src/api/exec_common.rs Manages async/sync edge transitions.
mssql-odbc/src/api/driver_connect.rs Caches negotiated server version.
mssql-odbc/src/api/close_cursor.rs Restores async mode before draining.
mssql-mock-tds/src/query_response.rs Models additional result sets.
mssql-mock-tds/src/protocol.rs Serializes multi-result batches.

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

Comment thread mssql-odbc/src/handles/stmt.rs Outdated
self.current_row = None;
self.row_batch.clear();
self.spare_rows.clear();
self.pending_fetch_error = None;
Comment on lines +839 to +841
for (idx, set) in sets.into_iter().enumerate() {
serialize_result_set(&mut result, set, idx == last);
}
.client
.as_ref()
.and_then(|c| c.server_version())
.server_version
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as draft August 10, 2026 06:49
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as ready for review August 10, 2026 07:40
@saurabh500
Saurabh Singh (saurabh500) marked this pull request as draft August 10, 2026 13:28
Drive SQLFetch/SQLGetData through the blocking TdsSyncClient edge (no per-row block_on), with chunked SQLGetData for PLP varchar(max)/nvarchar(max), zero-copy narrow passthrough, and zero-alloc scalar/decimal writers. Batch-fetch machinery removed so single-row fetch is the only path.
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.

2 participants