FIX: dispatch output converters on integer ODBC SQL type codes (#684) - #690
Merged
Conversation
add_output_converter documents an integer ODBC SQL type key, but dispatch keyed on the Python type in cursor.description[i][1], so integer keys silently never fired. Store per-column raw SQL type codes and dispatch on the integer code first, then Python type, then the legacy WVARCHAR catch-all. Also build the converter map for catalog metadata result sets so converters apply consistently.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes output-converter dispatch so Connection.add_output_converter(sqltype, func) honors integer ODBC SQL type codes (pyodbc-compatible) instead of only firing for Python-type keys derived from cursor.description[i][1]. This aligns runtime behavior with the documented API and resolves the silent no-op described in #684.
Changes:
- Preserve raw ODBC SQL type codes per column and use them first when building the per-result-set output-converter dispatch map (with Python-type and legacy
SQL_WVARCHARfallback preserved). - Ensure catalog/metadata result sets (e.g.,
cursor.tables()) also build a converter map so converters apply consistently. - Update the public type surface (
connection.pydoc/signature andmssql_python.pyi) and add a regression test covering integer-key dispatch semantics and precedence.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
mssql_python/cursor.py |
Store per-column raw ODBC type codes and dispatch converters in pyodbc-compatible precedence order; build converter maps for metadata result sets. |
mssql_python/connection.py |
Broaden add_output_converter to accept Union[int, type] and correct/clarify converter semantics in the docstring. |
mssql_python/mssql_python.pyi |
Update stub signature for add_output_converter to match runtime API. |
tests/test_003_connection.py |
Add regression test covering integer SQL type keys, precedence, exact-type matching, NULL handling, string converter path, and metadata behavior. |
…ispatch test_row_output_converter_general_exception registered a converter under integer SQL type 12 (SQL_VARCHAR), which previously never fired due to the GH #684 dispatch bug. Now that integer keys dispatch correctly, the converter fires and receives UTF-16LE bytes, so the old value=="test_value" guard no longer matches. Make the converter raise unconditionally to faithfully exercise the "converter raised -> keep original value" path, and move the converter restore into finally so a failed assertion can never leak the converter onto the shared connection (which was cascading into 6 unrelated VARCHAR test failures).
Assert user-visible behavior (a catalog string column is actually passed through the registered SQL_WVARCHAR converter) instead of the private cursor._cached_converter_map cache field, which was brittle to internal refactors. tables() row string columns now come back as "CONV:..." while NULL stays None.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.3%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 83.7%
mssql_python.connection.py: 84.7%🔗 Quick Links
|
…684) Adds test_execute_describe_col_exception_resets_description_and_sql_types, which patches ddbc_bindings.DDBCSQLDescribeCol to raise during execute() and asserts both self.description and self._column_sql_types are reset to None. Seeds a real SELECT first so the reset has a populated SQL-type list to clear. Covers the previously-uncovered defensive reset line in the execute() describe-failure except branch (diff coverage gap flagged by the coverage bot).
subrata-ms
reviewed
Jul 30, 2026
subrata-ms
reviewed
Jul 30, 2026
… (GH #684) Address review feedback on add_output_converter (Union[int, type] key): - Clarify docstring that the key is not validated for pyodbc compatibility; a key matching neither an exact integer ODBC SQL type code nor an exact materialized Python type (e.g. set, dict, object, a decimal.Decimal subclass) is stored but never dispatched -- a harmless no-op rather than an error. Enumerate the supported Python types. - Correct the NULL contract: the converter is not invoked for SQL NULL; the value is returned as None unchanged. - Add six live-DB tests: - one decimal.Decimal converter covers DECIMAL/NUMERIC/MONEY/SMALLMONEY, and an integer SQL_DECIMAL (code 3) key exact-matches MONEY/SMALLMONEY but not NUMERIC (code 2); - datetime and bytes Python-type converters; - SQL NULL skips the converter (spy asserts it is never called); - a mixed int/decimal/str/datetime result set routes each column to its own type-keyed converter; - unsupported keys (set/object/Decimal subclass) are no-ops (spies assert never invoked, values pass through unchanged); - the cached converter map is reused consistently across a multi-row fetchall.
subrata-ms
approved these changes
Jul 31, 2026
subrata-ms
approved these changes
Jul 31, 2026
sumitmsft
approved these changes
Aug 3, 2026
sumitmsft
left a comment
sumitmsft
left a comment
Contributor
There was a problem hiding this comment.
looks good to me. Approved.
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.
Work Item / Issue Reference
Summary
Connection.add_output_converter(sqltype, func)documents (and pyodbc accepts) aninteger ODBC SQL type code as the key — e.g.
SQL_DECIMAL,SQL_INTEGER. However,dispatch in
Cursor._build_converter_map()keyed on the Python type stored incursor.description[i][1](via_map_data_type), so integer-keyed converters were storedbut silently never fired. Only Python-type keys (e.g.
decimal.Decimal,str) worked.This makes the driver diverge from pyodbc and from its own documentation.
Root cause
The raw ODBC SQL type code (
col["DataType"]) is available when the description is builtin
_initialize_description, but it was discarded._build_converter_mapthen looked upconverters using
description[i][1](a Python type), which can never match an integer key.Fix
cursor.py_initialize_descriptionnow also storesself._column_sql_types— the raw ODBC SQLtype codes, parallel to
description(reset toNonewhen there are no columns)._build_converter_mapdispatches in pyodbc-compatible order: (1) integer SQL typecode, (2) Python type in
description[i][1], (3) the legacyWVARCHARcatch-all.This is purely additive — existing Python-type converters keep working.
_prepare_metadata_result_setnow builds the converter map so catalog metadata resultsets (
columns(),tables(), …) honor converters consistently with normal result sets.self._column_sql_types = Noneresets alongside the bareself.description = Noneassignments (execute/executemany describe-fail paths and
nextset).connection.py—add_output_convertersignature is nowsqltype: Union[int, type]; thedocstring is corrected to describe both key styles (integer key takes precedence) and the
value semantics (already-materialized Python object;
strpassed as UTF‑16LE bytes).mssql_python.pyi— stub updated to match (Union[int, type]).Exact-type dispatch
Integer keys use exact ODBC type matching, so
SQL_DECIMALandSQL_NUMERICaredistinct (they are not collapsed to
decimal.Decimal), matching pyodbc.Tests
Added
tests/test_003_connection.py::test_output_converter_integer_sql_type_key_gh684covering: integer key fires; exact-type dispatch (
SQL_INTEGERdoes not touchDECIMAL,but fires on
INT); integer-key precedence over a Python-type key; distinctDECIMALvsNUMERICconverters; NULL staysNone; theSQL_WVARCHARstring path (converter receivesUTF‑16LE bytes); and metadata result sets building a converter map.
Validation
black --check --line-length=100clean on all changed files.