Rewire mssql-odbc fetch hot path onto the reactor-free sync core (L5) - #204
Draft
Saurabh Singh (saurabh500) wants to merge 1 commit into
Conversation
Saurabh Singh (saurabh500)
force-pushed
the
dev/saurabh/rewire-odbc-sync-core-l5
branch
from
August 10, 2026 03:09
87e2e72 to
5c09f0e
Compare
Saurabh Singh (saurabh500)
marked this pull request as ready for review
August 10, 2026 03:21
Saurabh Singh (saurabh500)
marked this pull request as draft
August 10, 2026 03:26
Saurabh Singh (saurabh500)
marked this pull request as ready for review
August 10, 2026 06:38
Copilot started reviewing on behalf of
Saurabh Singh (saurabh500)
August 10, 2026 06:39
View session
Contributor
There was a problem hiding this comment.
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.
| 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 |
Saurabh Singh (saurabh500)
marked this pull request as draft
August 10, 2026 06:49
Saurabh Singh (saurabh500)
marked this pull request as ready for review
August 10, 2026 07:40
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.
Saurabh Singh (saurabh500)
force-pushed
the
dev/saurabh/rewire-odbc-sync-core-l5
branch
from
August 10, 2026 21:17
5c09f0e to
01f5ffa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
L5 — Rewire mssql-odbc fetch onto
TdsSyncClientStacked on the frozen L4 tip (
dev/saurabh/sans-io-expose-l4-tdssyncclient@36bc4648). Append-only; base = L4 branch, notmain.What this does
Plaintext raw-TCP connections now serve
SQLFetchthrough the reactor-freeTdsSyncClient(blockingstdreads, no tokio reactor). TLS and other non-raw transports fall back to the untouched asyncblock_onpath, byte-identically — the speedup is opportunistic on plaintext with zero user-visible flag.Ownership machine
DbcClient { Async(TdsClient) | Sync(TdsSyncClient) }in the connection state.finish_execute), persisting across allSQLFetchcalls.into_async) atSQLMoreResults/ close / free before driving the existingadvance/close_query.into_sync()runs insidedbc.runtime.enter()so the runtime handle is captured for the later revert (a nakedinto_sync()on the bare ODBC thread would captureNoneand poisoninto_async). Implemented in one place:flip_to_fetch_edge.Batch buffer (batch-ready, per-row default)
SYNC_FETCH_DEFAULT_MAX_ROWS(default 1 = per-row byte-identical INFO/SWI parity; flip to 64 is a one-line change).max_rows > 1, buffered rows are served before the error surfaces, reproducing the async row-then-error ordering.Other
server_versionat connect soSQL_DBMS_VERno longer reads the live client while a sync cursor is open.#[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 theSQLMoreResultsinterleave 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 externaltests/crate can't link it or reachpub(crate)entry points)SQLFetch@ max_rows=1 == async oracle, byte-identicalSQLMoreResultsinterleave: sync-fetch →into_async→advance→ re-into_sync, both sets verifiedflip_to_fetch_edgekeeps the DBC on the async edgeGates:
cargo bfmtclean,cargo bclippy(-D warnings) clean,cargo nextest -p mssql-odbc= 472 passed (468 existing + 4 new),mssql-mock-tds16 passed.Part of the sans-I/O native stack - see cover PR #189 for the stack-level summary and tracked debt.