Stage 4: auth/recovery, ODBC tests, mssql-python e2e (ADO #47340) - #302
Draft
Saurabh Singh (saurabh500) wants to merge 2 commits into
Conversation
…2e plan Confirm SQLSetConnectAttr(SQL_ATTR_RESET_CONNECTION) and TdsClient::reset_connection() preserve the auth/recovery context (same physical login, no re-auth), documented on reset_connection and locked in by a test rejecting a post-connect access token (B6). Fill remaining ODBC unit gaps: CONNECTION_DEAD reports dead for a client marked known-dead. Add a live C++ e2e (connection_pool_test.cpp) mirroring transaction_test.cpp: same-physical-connection reuse, clean state for the next borrower, isolation reset each checkout, prepared statement survival, and value validation. Document the mssql-python end-to-end verification plan (odbc-connection-pooling-python-e2e.md) since no in-repo Python pool harness exists; reference it from the main plan. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
Saurabh Singh (saurabh500)
added
enhancement
The reprepare-after-reset test asserts mssql-odbc-specific behavior: our driver transparently re-prepares after sp_reset_connection drops the server-side handle, while msodbcsql blindly reuses the dropped handle and fails (native 8179). Add SKIP_IF_COMPARING_MSODBCSQL() so the parity comparison leg skips this intended divergence, mirroring transaction_test.cpp. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
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
1 participant
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.
Stage 4 (final) of the mssql-odbc connection-pooling effort (ADO User Story #47317). Implements auth/recovery hardening, fills remaining ODBC test gaps, adds a live C++ e2e for pool reuse, and documents the mssql-python end-to-end verification plan.
Stacked on Stage 3 PR #301 (=> #296 => #294 => #293). Base branch:
saurabh500-stage-3-reset-acking-isolation(NOT main).Work item: AB#47340.
B6 — Auth / recovery context across reset (CONFIRMATION, no behavior change)
Traced the connect + reset paths.
SQLSetConnectAttr(SQL_ATTR_RESET_CONNECTION)andTdsClient::reset_connection()are the same physical login: no LOGIN7/fedauth re-run, no token re-send, andon_reset_connection_ack()/restore_login_defaults()never touchrecovery_context.client_context(auth state). A new/rotated access token only enters at connect time; settingSQL_COPT_SS_ACCESS_TOKENafter connect is rejected (HY011), so a rotated token forces a new physical login, never a live-session re-auth. This was a confirmation — the only changes are a doc comment onTdsClient::reset_connectionand a lock-in unit test (access_token_after_connect_is_rejected).B7 — ODBC tests
connection_dead_reports_true_when_client_marked_dead(a connected-but-poisoned client readsSQL_CD_TRUE). Existing coverage already had connected/disconnected/never-connected, value validation, busy rejection, poison-on-failure, and isolation re-apply.mssql-odbc/tests/e2e/tests/connection_pool_test.cpp, mirroringtransaction_test.cppstructure and gating (HasConnection()), registered in CMakeLists. Borrower A mutates isolation/temp-table/txn/database; the pool checks in + resets; borrower B on the same @spid sees clean login defaults (no temp table,@@TRANCOUNT=0, isolation back to READ COMMITTED, original DB), plus prepared-statement survival, per-checkout isolation reset, andCONNECTION_DEADstaying FALSE on a healthy connection.Workstream C — mssql-python e2e
No in-repo mssql-python/Python-pool harness exists, and adding one would pull heavy Python +
mssql-pythoninfra intomssql-rs(against the "no second pool" non-goal). Per the plan's guidance, added a concise verification plandocs/odbc-connection-pooling-python-e2e.mdcovering scenarios (a)–(f) — same-connection reuse, dead-connection discard, prepared-statement invalidation, concurrent checkout, token-identity separation + near-expiry reconnect, and the #343 isolation-reset — using the existing driver-registration hook (run_e2e.ps1/.sh). Linked from the main plan doc.Definition of Done status
SQL_ATTR_CONNECTION_DEADcached, no probeSQL_ATTR_RESET_CONNECTIONarms real reset; clean fail when disconnected/busySQL_ATTR_TXN_ISOLATIONreal server change incl. reset to READ COMMITTEDcargo bfmt,cargo bclippy, focused suites passValidation
cargo bfmt✅cargo bclippy✅ (warnings-as-errors)mssql-odbclib nextest: 578 passed. Targeted pooling/reset/auth unit tests: pass. TDS reset-ack unit tests: pass.mssql-tdslive reset tests require a live SQL Server (SQL_PASSWORD/server), unavailable in this environment — they are gated and left for live/CI. No new failures introduced.Non-goals kept intact
No second Rust pool; no live-session token refresh; no raw-T-SQL isolation reset (shared, documented limitation matching msodbcsql/mssql-python #343).