FEAT: Add opt-in/opt-out ODBC provider selection (msodbcsql18 / mssql-odbc)

[gargsaumya]

Linked work item: AB#47445

Summary

This pull request introduces opt-in/opt-out support for selecting which native ODBC provider is loaded by mssql-python, allowing users to choose between the default Microsoft ODBC Driver 18 (msodbcsql18) and the Rust-based driver (mssql-odbc), if the latter is installed. The selection can be made via a module property or environment variable before the first connection, with diagnostics and warnings for precedence and immutability. The implementation includes a new provider manager, updates to documentation, and improvements to driver loading and shutdown safety.

Native ODBC provider selection and diagnostics:

• Added support for selecting the native ODBC provider via the new mssql_python.native_provider module property or the MSSQL_PYTHON_NATIVE_PROVIDER environment variable (the env var takes precedence). The provider selection is resolved and frozen at the first connection, with warnings for conflicting or late assignments. The default remains "msodbcsql18", but opt-in to "mssql-odbc" is supported if the mssql-python-rs package is installed. [1] [2] [3] [4] [5]

• Introduced get_native_provider_info() for diagnostics, reporting the selected provider, source, package, version, driver path, and whether the selection is frozen. [1] [2]

Driver loading and connection logic:

• Updated connection logic so the ODBC provider is resolved and frozen only after all Python-side validation succeeds and just before the native driver loads, preventing premature provider locking on failed connection attempts. [1] [2]

• Ensured that enabling connection pooling does not prematurely resolve or freeze the ODBC provider, so explicit pooling configuration does not lock in the default provider before selection.

Documentation updates:

• Updated README.md and CHANGELOG.md to document the new provider selection mechanism, its usage, precedence rules, and diagnostic interface. [1] [2]

Type hints and interface improvements:

• Updated type hints in mssql_python.pyi to reflect the new native_provider property and get_native_provider_info() function. [1] [2]
• Minor import and typing improvements in mssql_python/__init__.py.

Native extension (C++ backend) safety:

• Improved Python shutdown/finalization detection in the C++ extension to use thread-safe, GIL-free APIs (Py_IsFinalizing or _Py_IsFinalizing), preventing crashes during interpreter shutdown from foreign threads. [1] [2]

[Copilot]

Pull request overview

Adds a process-wide, resolve-once ODBC provider selection mechanism so mssql-python can switch at runtime between the classic ODBC Driver 18 provider (msodbcsql18) and a future Rust provider (mssql-odbc), while keeping the existing public connection API unchanged.

Changes:

• Introduces ProviderManager to resolve provider selection (env var → module property → default), freeze it at first resolution, and fail closed on invalid selections.
• Wires provider resolution into Connection.__init__ and pushes the selected provider into the native loader via ddbc_bindings.set_odbc_provider.
• Adds a new unit test module covering precedence/normalization/freeze behavior and missing-provider fail-closed behavior.

Reviewed changes

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

Show a summary per file

File	Description
tests/test_026_odbc_provider.py	Adds unit tests for provider precedence, normalization, freeze semantics, and fail-closed behavior.
mssql_python/pybind/ddbc_bindings.cpp	Adds native-side provider selection plumbing and uses provider-specific package/dist names during driver resolution.
mssql_python/odbc_provider.py	Implements the Python-side provider selection engine (ProviderManager).
mssql_python/connection.py	Freezes/verifies provider selection and pushes it into the native layer before driver load.
mssql_python/init.py	Exposes mssql_python.odbc_provider and get_odbc_provider_info() as public diagnostics/surface area.

---

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

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 91%

🎯 Overall Coverage 82%

📈 Total Lines Covered: 7953 out of 9646 📁 Project: mssql-python

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql_python/init.py (100%)
• mssql_python/connection.py (100%)
• mssql_python/odbc_provider.py (96.8%): Missing lines 100,102,209,211
• mssql_python/pybind/ddbc_bindings.cpp (88.2%): Missing lines 1008-1011,1106,1108,1147,1244,1535-1537
• mssql_python/pybind/logger_bridge.cpp (42.9%): Missing lines 171-172,179-180

Summary

• Total: 237 lines
• Missing: 19 lines
• Coverage: 91%

mssql_python/odbc_provider.py

Lines 96-106

   96             env_value = os.environ.get(NATIVE_PROVIDER_ENV_VAR)
   97             if canonical is not None and env_value and env_value.strip():
   98                 try:
   99                     env_provider = _normalize(env_value)
! 100                 except ValueError:
  101                     # Preserve the existing fail-closed error at connection time.
! 102                     return
  103                 if canonical != env_provider:
  104                     cls._warn_env_override(canonical, env_provider)
  105 
  106     @classmethod

---

Lines 205-215

  205 
  206                 driver_path = ddbc_bindings._get_odbc_driver_path(
  207                     os.path.dirname(os.path.abspath(module_file)), provider
  208                 )
! 209         except Exception:  # pylint: disable=broad-exception-caught
  210             # Diagnostics must remain safe even for a broken provider package.
! 211             pass
  212 
  213         info: Dict[str, object] = {
  214             "id": provider,
  215             "package": package,

---

mssql_python/pybind/ddbc_bindings.cpp

Lines 1004-1015

  1004 // verify the external package actually ships this platform's driver binary.)
  1005 std::string GetDriverPathCpp(const std::string& moduleDir);
  1006 std::string GetDriverPathForProviderCpp(const std::string& moduleDir,
  1007                                         const std::string& providerId);
! 1008 
! 1009 // -----------------------------------------------------------------------------
! 1010 // ODBC provider selection
! 1011 //
  1012 // Two providers are supported: the classic Microsoft ODBC Driver 18
  1013 // ("msodbcsql18", shipped by mssql_python_odbc) and the Rust driver
  1014 // ("mssql-odbc", shipped inside mssql_py_core / the mssql-python-rs wheel).
  1015 // Python is the sole

---

Lines 1102-1112

  1102     // hard crash.
  1103     py::gil_scoped_acquire gil;
  1104     const std::string providerId = GetSelectedProviderId();
  1105     const std::string packageName = ProviderPackageForId(providerId);
! 1106     const std::string distName = ProviderDistForId(providerId);
  1107     try {
! 1108         py::object module = py::module::import(packageName.c_str());
  1109         py::object module_path = module.attr("__file__");
  1110         std::string module_file = module_path.cast<std::string>();
  1111 
  1112         fs::path parentDir = fs::path(module_file).parent_path();

---

Lines 1143-1151

  1143                 packageName.c_str(), parentDir.string().c_str());
  1144             ThrowStdException(
  1145                 "The '" + distName + "' package is installed but its ODBC driver binaries "
  1146                 "are missing or incomplete for this platform. Reinstall it with: "
! 1147                 "pip install --force-reinstall " + distName);
  1148         }
  1149         LOG("GetOdbcLibsBaseDir: Using external %s package - directory='%s'",
  1150             packageName.c_str(), parentDir.string().c_str());
  1151         return parentDir.string();

---

Lines 1240-1248

  1240  * dependencies during critical initialization, ensuring compatibility across
  1241  * all supported platforms.
  1242  */
  1243 std::string GetDriverPathForProviderCpp(const std::string& moduleDir,
! 1244                                         const std::string& providerId) {
  1245 #if !defined(MSODBCSQL_VERSION_MAJOR) || !defined(MSODBCSQL_VERSION_MAJOR_MINOR)
  1246 #error \
  1247     "MSODBCSQL_VERSION_MAJOR / MSODBCSQL_VERSION_MAJOR_MINOR must be defined at build time. " \
  1248     "They are derived from mssql_python_odbc.__version__ in CMakeLists.txt so the driver " \

---

Lines 1531-1541

  1531         std::rethrow_exception(m_loadError);
  1532     }
  1533 }
  1534 
! 1535 bool DriverLoader::isDriverLoaded() const {
! 1536     return m_driverLoaded.load();
! 1537 }
  1538 
  1539 // SqlHandle definition
  1540 SqlHandle::SqlHandle(SQLSMALLINT type, SQLHANDLE rawHandle) : _type(type), _handle(rawHandle) {}

---

mssql_python/pybind/logger_bridge.cpp

Lines 167-176

  167     // gil_scoped_acquire below) instead of being dropped. Py_IsFinalizing() is
  168     // public since 3.13; _Py_IsFinalizing() is the exported CPython 3.7+ call it
  169     // wraps, and is what pybind11 itself uses for the same purpose.
  170     if (Py_IsInitialized() == 0) {
! 171         return;
! 172     }
  173 #if PY_VERSION_HEX >= 0x030D0000
  174     if (Py_IsFinalizing()) {
  175         return;
  176     }

---

Lines 175-184

  175         return;
  176     }
  177 #else
  178     if (_Py_IsFinalizing()) {
! 179         return;
! 180     }
  181 #endif
  182 
  183     // Format the message
  184     va_list args;

---

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 75.6%
mssql_python.row.py: 77.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.6%
mssql_python.pybind.connection.connection.cpp: 84.4%
mssql_python.logging.py: 85.5%
mssql_python.helpers.py: 89.3%
mssql_python.pooling.py: 90.1%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

[Vahid (Vahid-b)]

Summary

Adds a process-wide, resolve-once selector between msodbcsql18 and the Rust mssql-odbc driver, with msodbcsql18 as the Phase 1 default and the Rust path failing closed until its wheel ships. The design is sound — Python as the sole resolver, the native side purely a receiver, freeze at first connect, no public API change. One finding I'd treat as blocking; the rest are suggestions and nits.

I verified the load ordering the whole design rests on: loadDriver() is lazy behind std::call_once, and the only entry into it from a fresh process is the Connection constructor at pybind/connection/connection.cpp:25. connection.py:375-376 pushes the provider well before ddbc_bindings.Connection(...) at line 741. That part is correct.

All of copilot-pull-request-reviewer's findings and the DevSkim alert look addressed in e9372ba7 / 094dc2f3 / f854f8dc; I am not re-filing any of them. The one that is only half-closed is the Optional[str] stub, noted inline.

Blocking

The pooling hook (pooling.py:66-71) — its stated reason is false, and its only real effect is an unwanted early freeze. Detail inline. enable_pooling does not load the native driver, so the hook protects nothing, but it does make mssql_python.pooling() freeze the provider and become able to raise ImportError.

Suggestions

Six inline: the source field that is always None before the freeze, effective() raising on a bad env var, the native side's silent coercion plus unguarded set_odbc_provider, the mssql-auth.dll justification, the Linux path layout versus the libc split, and the Optional[str] stub.

One that has no line to sit on: no test covers the pooling freeze path. That is where the blocking finding lives, and a test asserting PoolingManager.enable() does not freeze the selection would have caught it. Worth adding alongside the fix.

Nits

Three inline: the mid-file #include, the recwarn assertion, and the resolve-before-validate ordering in Connection.__init__. Plus one that spans the change rather than a line: NormalizeProviderId (ddbc_bindings.cpp:993-1003) strips interior whitespace while Python's _normalize (odbc_provider.py:44-54) only does .strip(). Harmless while Python is the sole resolver, but the two should agree if the native one ever becomes authoritative.

What I ran

pytest isn't installed here and the package needs the compiled ddbc_bindings extension, so I could not run the suite — CI is the gate for that. The two runtime observations below came from loading odbc_provider.py standalone against a stubbed mssql_python.logging:

A. get_info BEFORE resolve, env=mssql-odbc: {'id': 'mssql-odbc', 'package': 'mssql_python_rust_odbc', 'source': None, 'frozen': False}
C. get_info AFTER  resolve:                 {'id': 'mssql-odbc', 'package': 'mssql_python_rust_odbc', 'source': 'environment', 'frozen': True}
D. get_info    with bad env RAISED: ValueError Unknown ODBC provider 'bogus-value'. ...
E. effective() with bad env RAISED: ValueError Unknown ODBC provider 'bogus-value'. ...

The cross-repo claims (mssql-auth.dll, artifact filenames, the Linux libc split) were checked against microsoft/mssql-rs source rather than from memory; file and line are cited in each comment.

---

Reviewed with GitHub Copilot on behalf of Vahid (@Vahid-b), then checked by hand. Not an approval — push back on anything that looks wrong.

[Saurabh Singh (saurabh500)]

The native load-order issue below blocks the Rust opt-in. I also agree with the existing PoolingManager.enable() thread: native enable_pooling() only configures pool state, so it should not freeze provider selection when the documented boundary is the first connection. I did not duplicate that thread.

The authentication comment below reflects the clarified requirement that the Rust provider also requires mssql-auth.dll for its Windows interactive-authentication path.

[Gaurav Sharma (bewithgaurav)]

was skimming through, and found a couple of cases worth thinking about - putting as a comment review

[gargsaumya]

Copilot resolve the merge conflicts in this pull request

[Copilot]

Copilot resolve the merge conflicts in this pull request

Merged main into the branch in 03880cf. The only conflict was in mssql_python/pybind/ddbc_bindings.cpp, in the Windows mssql-auth.dll preload: main hardened it to LoadLibraryExW with LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32 | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR, while this branch wrapped that block in the kProviderMsodbcsql18 guard. Resolved by keeping both: the hardened call inside the provider guard. tests/test_026_windows_dll_search.py from main (no bare LoadLibraryW, constrained flags on every LoadLibraryExW call) passes against the merged source.

[Copilot]

🟡 Changes recommended

Native provider selection has a race that can allow provider mutation after the driver loads, which can break the intended process-wide immutability guarantees.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Critical provider-state and shutdown-race issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

tests/test_026_odbc_provider.py:132

• This test only verifies a synthesized filename; it never loads mssql-odbc or opens a connection through it. Consequently an incorrect package root, missing transitive library, ABI/load failure, or absent ODBC export would all pass while the advertised provider is unusable. Since validation already installs mssql_py_core and runs against SQL Server, add an isolated integration case that selects mssql-odbc, connects, and executes a simple query.

mssql_python/pybind/ddbc_bindings.cpp:6422

• Making driver loading lazy means tests/test_015_utf8_path_handling.py::test_module_import_exercises_path_handling no longer exercises GetOdbcLibsBaseDir, LoadDriverLibrary, or the Windows auth-DLL path as that test claims. Add or update a test to trigger the first connection/load from the non-ASCII-path scenario so this change does not silently remove regression coverage for UTF-8 driver paths.

    // Deliberately do NOT call loadDriver() here: doing so would resolve and

• Files reviewed: 12/12 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Provider validation and concurrency defects must be fixed, and Rust-provider integration coverage added.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

mssql_python/odbc_provider.py:158

• effective() and resolve() use separate critical sections, with the provider import between them. A concurrent property change can therefore make this method validate one package but freeze another (for example, validate the default package, then freeze mssql-odbc without checking that mssql_py_core exists). This also defeats the guarantee that a missing provider does not freeze selection. Snapshot and validate a candidate, then reacquire the lock and freeze it only if the effective selection still matches; otherwise retry.

        provider = cls.effective()
        package = _PACKAGE_BY_PROVIDER[provider]
        try:
            importlib.import_module(package)

• Files reviewed: 12/12 changed files
• Comments generated: 2
• Review effort level: Balanced