Rewire mssql-py-core onto the reactor-free sync core (opt-in sync cursor)

[Saurabh Singh (saurabh500)]

Description

L6 — Rewire mssql-py-core onto the reactor-free sync TDS core.

Framing (correctness/architecture, not a perf win yet): This layer (and the whole L-stack) is a correctness/architecture refactor — one protocol core with sync and async shells that deletes the per-row block_on tax and the fast::* hack. It is NOT a native-beating performance win yet. VarcharMax/PLP (~818× on both the async-before and sync-after variants, pre-existing), the Decimal/DateTime2 conversion gaps, and the per-column SQLGetData conversion/alloc path are the real performance debt to burn down next. It earns its keep on architecture and correctness, and it sets up the perf work rather than delivering it.

Stacked on the frozen L5 tip (dev/saurabh/rewire-odbc-sync-core-l5 @ 5c09f0e4). Append-only single commit; base = L5 branch, not main. Scope is mssql-py-core only — all frozen crates (mssql-tds, mssql-odbc, mssql-mock-tds, mssql-js, mssql-tds-cli) are empty-diff.

What this does

The Python bindings now share ONE protocol core across two cursor surfaces via a flip-in-place ownership cell:

• enum PyClient { Async(TdsClient) | Sync(TdsSyncClient) | Transitioning | Dead(String) } behind Arc<std::sync::Mutex<PyClient>> (pyclient.rs). The Arc points at the cell, so Arc::try_unwrap is never used — cursors keep their clone and match the enum under the lock.
• Flip is std::mem::replace(&mut *guard, Transitioning) → call into_sync/into_async on the moved-out value → store the result variant back. Transitioning is a sentinel that only exists between take and store (no early-return while transitioning); an unrecoverable flip stores Dead(String) and surfaces Err.
• Runtime-context hard rule: every into_sync() runs inside handle.enter() so the runtime handle is captured for the later revert (a naked into_sync() off-runtime would capture None and poison into_async).
• Revert-before-control-plane: revert to Async at end-of-rows / before close_query / before any control-plane op, mirroring L5.

Cursor surfaces (mode A for this layer)

• PyCoreSyncCursor (new): reactor-free row-pull. Plaintext connections flip to the Sync(TdsSyncClient) arm and drive next_row_into with no block_on; TLS/non-eligible transports fall back to the block_on-over-async path, byte-identically.
• PyCoreCursor (existing): kept exactly as-is — block_on-over-async backed. Public Python API unchanged — no regression. This is not a first-class async cursor: its block_on blocks the Python event loop. The genuine coroutine (async def / awaitable) cursor is L7, not this layer.
• Connection.cursor() / Connection.sync_cursor() expose the two surfaces.
• execute / COLMETADATA / advance / close / DML / bulkcopy / auth all stay async (control-plane rule); the sync flip is for the pure SELECT row-pull hot loop only, per result set.

rowcount

Additive rowcount captured from last_rows_affected() pre-flip on the async arm (faithful-count parity contract, populated by the shared apply_row_read_token count_map at L4 B′/L5). No independent count logic.

Scope decision (mode A) — real coroutine async cursor is L7

This layer ships mode A: sync arm reactor-free + the existing cursor retained as block_on-over-async. A genuinely first-class coroutine async cursor (pyo3-async-runtimes / future_into_py driving next_row_into().await on the Async arm, event-loop driven, non-blocking) is a large net-new public surface and lands as its own reviewable layer L7 stacked on top of this one. No pyo3-async-runtimes dependency is added here; Cargo.lock is unchanged.

Build / test path

mssql-py-core is edition 2024 and excluded from the cargo workspace, so the plain cargo b* aliases do NOT cover it — validated via scripts\bfmt.ps1 + scripts\bclippy.ps1 (both cover py-core) + maturin + the Python suite through the existing conftest.py fixtures.

Tests

• Sync-cursor fetch on a plaintext connection (reactor-free sync path).
• Sync-cursor TLS-fallback (block_on-over-async path) — env-skips locally on the pre-existing mock-TLS-on-Windows timeout; CI-validated.
• Attribute-set parity: the new sync cursor and the existing block_on-backed cursor expose the same surface.
• Byte parity: sync fetchone results identical to the block_on path.
• Error-mid-fetch sync recovery: mid-stream error reverts via into_async drain, connection reused (validated live: SQL 245 conversion error mid-stream).
• DML rowcount + multi-row SELECT parity validated against a live localhost SQL Server.

Gates: scripts\bfmt.ps1 clean, scripts\bclippy.ps1 (-D warnings) clean (workspace + py-core), maturin develop builds mssql_py_core + mssql_mock_tds, mock cursor suite = 10 passed / 1 env-skip.

Checklist

• cargo bfmt passes
• cargo bclippy passes
• cargo btest passes
• New/changed functionality has tests
• Public API changes are documented

---

Stack governance: DRAFT — the whole stack flips to ready-for-review together, bottom-to-top, after parent sign-off.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

Related work item / issue: Tracked as part of the sans-I/O native stack (#192)

[Copilot]

Pull request overview

Rewires Python bindings to share the async and reactor-free sync TDS core through an opt-in synchronous cursor.

Changes:

• Adds the shared async/sync client state machine.
• Introduces PyCoreSyncCursor and row-count support.
• Adds live-server and mock-server coverage.

Reviewed changes

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

Show a summary per file

File	Description
mssql-py-core/src/pyclient.rs	Implements shared client transitions and operations.
mssql-py-core/src/sync_cursor.rs	Adds the reactor-free sync cursor.
mssql-py-core/src/cursor.rs	Adopts the shared client and exposes rowcount.
mssql-py-core/src/connection.rs	Creates shared clients and sync cursors.
mssql-py-core/src/lib.rs	Registers the new Python class.
mssql-py-core/tests/test_sync_cursor.py	Adds live SQL Server tests.
mssql-py-core/tests/rs-only-tests/test_sync_async_cursor_mock.py	Adds mock-server path and transition tests.

---

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

[Saurabh Singh (saurabh500)]

Closing, consistent with the rest of the sans-I/O stack (#189–#203, closed 2026-08-13).

This PR is an L5–L7 consumer of the reactor-free sync core, so it has no base once that core is not landing. The stack was closed because the row-decode performance work that motivated it was re-scoped around #247, where benchmarked spikes showed the dominant win came from a far smaller change than inverting the core to a sync step() driver.

That argument has since been settled by measurement rather than projection. PR #264 has now merged (7a9c0b93), converting TdsPacketReader to RPITIT and removing per-read boxing — −61.6% decode time, independently corroborated at −61.4% on a separate harness built for another purpose. The win this stack was reaching for is realised, on ~12 draft PRs and ~8,400 lines less surface area.

Two further results from that work are worth recording here, because both cut against restructuring this path speculatively:

• Two follow-on optimizations (Precompute a per-column decode plan (ColumnDecodeSpec) and converge all decode sites on it #249, Converge decode onto decode_into via CaptureWriter #269) were built, measured, and refuted — each looked sound and one had already passed full CI. See Row decode performance: productionize PoC #238 across four axes (tracking) #247 for which metrics predicted decode throughput and which did not.
• Changes on this path must be measured on top of merged Remove per-row dispatch overhead from the row-decode path #264. Converge decode onto decode_into via CaptureWriter #269 measured neutral against pre-Remove per-row dispatch overhead from the row-decode path #264 main and became a +21.5% regression once rebased, because Remove per-row dispatch overhead from the row-decode path #264 removed the boxing that had been absorbing its cost.

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