ODBC connection pooling (ADO #47317) - #308
Draft
Saurabh Singh (saurabh500) wants to merge 12 commits into
Draft
Conversation
Design plan for enabling connection pool constructs in mssql-odbc so mssql-python's client-side pool can safely reuse a physical Rust ODBC connection. Covers reset/liveness/isolation semantics, msodbcsql cross-reference, and a 4-stage implementation breakdown. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
Route every ResetConnection ENVCHANGE through a single on_reset_connection_ack that resets session state, clears session-bound prepared caches (shared with the reconnect path), and restores the negotiated database/language/collation to their login values. Add TdsTransport::mark_known_dead and flag the transport dead when a server error token has severity/class >= 20, so a pool checkout discards a fatally errored connection even on a still-open socket. Add unit tests for the reset transition and the fatal-error path, plus a live test proving a prepared handle is invalidated across a reset. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
Add SQL_ATTR_CONNECTION_DEAD and SQL_ATTR_RESET_CONNECTION support for connection pooling (Stage 2, ADO #47338). - B1: add SQL_ATTR_CONNECTION_DEAD/SQL_CD_TRUE/SQL_CD_FALSE and SQL_ATTR_RESET_CONNECTION/SQL_RESET_CONNECTION_YES constants. - B2: SQLGetConnectAttr(SQL_ATTR_CONNECTION_DEAD) returns a cached, never-probe liveness read; disconnected/never-connected reads DEAD. - B3: SQLSetConnectAttr(SQL_ATTR_RESET_CONNECTION) validates the value (HY024 otherwise), rolls back a live local transaction, arms the full RESETCONNECTION bit via prepare_reset_connection(false), and clears local_tran_started; disconnected surfaces 08003, busy is rejected. - Unit tests cover both attributes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
Drive the RESETCONNECTION round trip eagerly inside SQLSetConnectAttr(SQL_ATTR_RESET_CONNECTION) so the reset is processed and acknowledged before pool checkout returns, rather than riding a later request that may short-circuit. A failed reset poisons the client and surfaces 08S01 so mssql-python discards the connection. Add TdsClient::reset_connection() (arms the bit and forces the ack round trip) and mark_connection_dead(); wire the ODBC handler to them, map failures to 08S01, and document the D9 raw-T-SQL isolation caveat at the same-value short-circuit. Cover the eager ack, poison, reset+isolation re-apply, and full checkout-cycle lifecycle with unit/mock tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
…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
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
Add PreparedStatementUsableAcrossReset, which runs on both drivers and verifies the shared contract: a prepared statement works before the reset and a freshly prepared statement works after it, so the connection stays usable across a pool reset on both mssql-odbc and msodbcsql. Narrow PreparedStatementSurvivesResetViaReprepare to just the mssql-odbc divergence: re-executing the same already-prepared handle after the reset must transparently re-prepare (msodbcsql fails native 8179). Keep SKIP_IF_COMPARING_MSODBCSQL() only on this test so the shared behavior is still compared on both parity legs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
This was referenced Aug 16, 2026
…n Windows The Windows ODBC Driver Manager decides whether to forward ODBC 3.8-only connection attributes (SQL_ATTR_RESET_CONNECTION) based on the driver's registry DriverODBCVer, read at load time, not the runtime SQLGetInfo(SQL_DRIVER_ODBC_VER). Without it the DM rejects the attribute with HY092 before reaching the driver, failing every connection_pool_test reset case on the Windows e2e legs. Register DriverODBCVer=03.80 (mirroring msodbcsql's sqlnclidrv.rgs) with matching snapshot/restore so cleanup unwinds it. Linux/unixODBC has no equivalent gate, so run_e2e.sh is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63e24621-7665-4572-a5b8-e5dfa55060a0
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql-odbc/src/api/txn.rsmssql-tds/src/connection/tds_client.rsmssql-tds/src/connection/transport/tds_transport.rs🔗 Quick Links |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Saurabh Singh (saurabh500)
marked this pull request as ready for review
August 16, 2026 17:43
Copilot started reviewing on behalf of
Saurabh Singh (saurabh500)
August 16, 2026 17:44
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Adds TDS and ODBC primitives required for safe physical-connection reuse by mssql-python’s client-side pool.
Changes:
- Centralizes TDS reset acknowledgement and fatal-error liveness handling.
- Adds ODBC reset, connection-dead, and isolation behavior.
- Adds unit, live E2E, and integration-verification coverage.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
mssql-tds/tests/test_reset_connection.rs |
Tests prepared-handle invalidation. |
mssql-tds/src/test_client_support.rs |
Adds reset and liveness test support. |
mssql-tds/src/connection/transport/tds_transport.rs |
Adds transport death marking. |
mssql-tds/src/connection/transport/network_transport.rs |
Implements death marking. |
mssql-tds/src/connection/tds_client.rs |
Handles reset acknowledgements and fatal errors. |
mssql-tds/src/connection/execution_context.rs |
Delegates reset state handling. |
mssql-odbc/tests/e2e/tests/connection_pool_test.cpp |
Adds live pooling tests. |
mssql-odbc/tests/e2e/run_e2e.ps1 |
Removes formatting noise. |
mssql-odbc/tests/e2e/CMakeLists.txt |
Registers pooling tests. |
mssql-odbc/src/api/txn.rs |
Implements eager reset and isolation flow. |
mssql-odbc/src/api/sqlstate.rs |
Adds communication-failure SQLSTATE. |
mssql-odbc/src/api/set_connect_attr.rs |
Routes reset attributes. |
mssql-odbc/src/api/odbc_types.rs |
Defines pooling constants. |
mssql-odbc/src/api/get_connect_attr.rs |
Reports cached connection liveness. |
docs/odbc-connection-pooling-python-e2e.md |
Documents Python integration verification. |
docs/odbc-connection-pooling-plan.md |
Describes pooling design and delivery plan. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Saurabh Singh (saurabh500)
enabled auto-merge (squash)
August 16, 2026 17:49
Saurabh Singh (saurabh500)
marked this pull request as draft
August 16, 2026 17:50
auto-merge was automatically disabled
August 16, 2026 17:50
Pull request was converted to draft
…rigor Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Implements Connection Pool constructs in
mssql-odbcfor ADO User Story #47317, somssql-python's client-side connection pool can safely reuse a physical Rust ODBC connection across borrowers.This is the single, consolidated PR for the feature (the earlier stacked stage PRs #293/#294/#296/#301/#302 have been closed in favor of this one). All four stages' work is included here against
main.What this delivers
TdsClient::on_reset_connection_ack(), which resets the session-state table, clears session-bound caches (prepared handles, param-encryption, pending captures) viaclear_session_bound_caches(), and restores negotiated DATABASE/LANGUAGE/COLLATION to login values. Fatal error tokens (severity >= 20) mark the transport dead viamark_known_dead()so a poisoned connection is discarded by the pool.SQL_ATTR_CONNECTION_DEAD(cached, never-probe liveness read, DEAD-until-proven) and the pooling reset attribute (value validation → HY024; 08003 when disconnected; busy rejection via claim-idle; rollback of a live local txn; arm the reset bit). The reset is accepted under both spellings — the ODBC 3.8SQL_ATTR_RESET_CONNECTION(116) and the msodbcsql vendor attributeSQL_COPT_SS_RESET_CONNECTION(1246) — so both the direct-loadingmssql-pythonconsumer and Driver-Manager-mediated callers can request a check-in reset (see below).TdsClient::reset_connection()arms RESETCONNECTION and drives a round trip that forces the ENVCHANGE ack (clearing session-bound caches); on failure the client is poisoned and08S01is surfaced somssql-pythondiscards it. TheSQL_ATTR_TXN_ISOLATIONhandler emits a realSET TRANSACTION ISOLATION LEVELbatch, becausesp_reset_connectiondoes not reset isolation (D9) — matchingmssql-pythonPR #343, which re-applies READ COMMITTED on every checkout.connection_pool_test.cppcovers same-SPID reuse, clean state for the next borrower, per-checkout isolation reset, connection-liveness, and prepared-statement behavior across reset. Shared behavior is verified on both drivers; the one intended mssql-odbc-specific divergence (transparent re-prepare of a dropped handle, which msodbcsql fails with native 8179) is isolated behindSKIP_IF_COMPARING_MSODBCSQL().Design doc
docs/odbc-connection-pooling-plan.md— full design: consumer analysis, discoveries D1–D10, workstreams, DoD, and non-goals.docs/odbc-connection-pooling-python-e2e.md— mssql-python pool integration verification plan (scenarios a–f) via the existingrun_e2e.ps1/run_e2e.shdriver-registration hook.Work items
Fixes AB#47317. Implements AB#47337, AB#47338, AB#47339, AB#47340.
Why the reset attribute is accepted under two identifiers (D10)
SQL_ATTR_RESET_CONNECTION(116) is an ODBC 3.8 attribute reserved for Driver Manager → driver communication: the DM sets it itself when returning a connection to its own pool, and per the 3.8 driver guidance "an application cannot set this attribute directly." The Windows DM enforces that — a DM-mediated application that sets it getsHY092("Option type out of range") from the DM before the call reaches any driver. This is not driver-specific: the installed msodbcsql18 fails identically. Such callers use msodbcsql's vendor attributeSQL_COPT_SS_RESET_CONNECTION(SQL_COPT_SS_BASE_EX+6= 1246, valueSQL_RESET_YES,odbcss.h) instead, since vendor-range attributes are passed straight through. unixODBC applies no such gate, so 116 reaches the driver on Linux/macOS.mssql-python— the consumer this feature exists for — is not subject to that gate, because it does not use a Driver Manager. It loads the driver library directly (LoadDriverLibrary()inmssql_python/pybind/ddbc_bindings.cpp:LoadLibraryWon Windows,dlopenelsewhere), binds the exports viaGetProcAddress/dlsym(SQLSetConnectAttr_ptret al.), and calls them itself. ItsSQL_ATTR_RESET_CONNECTION = 116therefore arrives at our exportedSQLSetConnectAttrWunchanged on every platform, Windows included — no upstream change is required.So the driver handles both identifiers on every platform: 116 for the direct-loading consumer, and 1246 for DM-mediated callers on Windows — which includes this repo's own C++ e2e suite, since it links against the Driver Manager.
connection_pool_test.cppselects the spelling its transport requires and passes against both mssql-odbc and msodbcsql.An earlier commit in this PR attempted to fix the Windows e2e
HY092by advertisingDriverODBCVer = "03.80"in the test registry registration. That was the wrong diagnosis — the DM's rejection is about who may set attribute 116, not about the driver's advertised ODBC version — and it did not fix the failure. That change has been reverted; noDriverODBCVerchange is needed, and this imposes no new requirement on a real install of the driver.Notes
Draft. Merge is author-owned.