Skip to content

FIX: Cursor.description returns dual-compatible SqlTypeCode type codes#355

Open
dlevy-msft-sql wants to merge 1 commit intomicrosoft:mainfrom
dlevy-msft-sql:Issue-352
Open

FIX: Cursor.description returns dual-compatible SqlTypeCode type codes#355
dlevy-msft-sql wants to merge 1 commit intomicrosoft:mainfrom
dlevy-msft-sql:Issue-352

Conversation

@dlevy-msft-sql
Copy link
Contributor

@dlevy-msft-sql dlevy-msft-sql commented Dec 1, 2025

Work Item / Issue Reference

GitHub Issue: #352


Summary

Fixes cursor.description[i][1] to return SqlTypeCode instances instead of raw Python type objects. SqlTypeCode is dual-compatible: it compares equal to both SQL integer type codes (DB-API 2.0) and Python types (pandas/polars compatibility), so existing code continues to work unchanged.

Problem:

polars.read_database() fails with ComputeError: could not append value: 2013-01-01 of type: date to the builder. The root cause is that cursor.description[i][1] returns Python type objects (str, int, datetime.date) instead of SQL integer type codes as required by DB-API 2.0 (PEP 249). Polars uses these type codes to determine column types, and Python type objects cause type-mapping failures.

Solution -- SqlTypeCode class (type_code.py):

  • SqlTypeCode wraps an ODBC integer type code with .type_code (raw SQL integer) and .python_type (mapped Python type) attributes
  • __eq__ matches both int values and Python type objects, so desc[1] == str and desc[1] == -9 both work
  • __int__ returns the raw SQL type code for DB-API 2.0 consumers
  • Intentionally unhashable (__hash__ raises TypeError with actionable guidance) because equality spans multiple hash domains

Cursor internals:

  • _column_metadata stores per-column (col_name, sql_type, col_size, precision, nullable) tuples from DDBCSQLDescribeCol
  • _build_converter_map refactored to use stored metadata instead of re-querying
  • _initialize_description builds SqlTypeCode from stored metadata

Converter API:

  • add_output_converter, get_output_converter, remove_output_converter in connection.py accept Union[int, SqlTypeCode, type]
  • Uses isinstance(sqltype, SqlTypeCode) to unwrap to .type_code before forwarding to native layer

Constants and C++ bindings:

  • Added SQL_SS_TIME2, SQL_SS_UDT, SQL_SS_XML constants
  • Added #define guards in ddbc_bindings.cpp for SQL_DATETIME2, SQL_SMALLDATETIME, SQL_SS_UDT

Changes

File Change
mssql_python/type_code.py NEW -- SqlTypeCode class extracted to own module
mssql_python/cursor.py _column_metadata tracking, _build_converter_map refactor, SqlTypeCode integration
mssql_python/connection.py Converter methods accept SqlTypeCode via isinstance
mssql_python/constants.py +3 constants (SQL_SS_TIME2, SQL_SS_UDT, SQL_SS_XML)
mssql_python/pybind/ddbc_bindings.cpp +3 #define guards
mssql_python/__init__.py Export SqlTypeCode
mssql_python/mssql_python.pyi Type stub for SqlTypeCode
CHANGELOG.md #352 entries
tests/test_002_types.py 18 new SqlTypeCode unit tests
tests/test_004_cursor.py Rewritten description tests, new metadata tests
tests/test_018_polars_integration.py NEW -- 5 polars integration tests

Breaking Changes

None. Full backward compatibility maintained:

  • cursor.description[i][1] == str still works (Python type comparison)
  • cursor.description[i][1] == -9 also works (SQL type code comparison)
  • int(cursor.description[i][1]) returns the raw SQL integer type code

Testing

  • 18 unit tests for SqlTypeCode (equality, hashing, repr, int conversion, pandas compatibility)
  • Cursor description tests rewritten with SqlTypeCode-aware assertions
  • Metadata thread safety and sequential isolation tests
  • 5 polars integration tests covering the original reported issue (polars.read_database())

Copilot AI review requested due to automatic review settings December 1, 2025 17:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes issue #352 by implementing two main improvements: (1) correcting cursor.description to return SQL type codes (integers) instead of Python type objects, ensuring DB-API 2.0 specification compliance, and (2) adding support for SQL Server spatial data types (geography, geometry, hierarchyid) by handling the SQL_SS_UDT type code (-151).

Key changes:

  • Fixed cursor.description[i][1] to return SQL type integer codes (e.g., 4, -9) instead of Python types (int, str) per DB-API 2.0 spec
  • Added SQL_SS_UDT (-151) support for SQL Server User-Defined Types including spatial data types
  • Updated output converter lookup to use SQL type codes consistently throughout the codebase

Reviewed changes

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

Show a summary per file
File Description
mssql_python/cursor.py Changed cursor.description to return SQL type codes instead of Python types; added _column_metadata storage; updated _build_converter_map to use SQL type codes; added mappings for UDT, XML, DATETIME2, SMALLDATETIME types
mssql_python/constants.py Added SQL_SS_UDT = -151 constant for SQL Server User-Defined Types
mssql_python/pybind/ddbc_bindings.cpp Added C++ constants for SQL_SS_UDT, SQL_DATETIME2, SQL_SMALLDATETIME; implemented UDT handling in SQLGetData_wrap, FetchBatchData, FetchMany_wrap, and FetchAll_wrap for LOB streaming
tests/test_004_cursor.py Added lob_wvarchar_column to test schema; updated test_cursor_description to expect SQL type codes; added comprehensive geography type test suite (14 new tests); separated LOB and non-LOB fetch tests; fixed output converter test for UTF-16LE encoding
tests/test_003_connection.py Simplified converter integration tests to use SQL type constants directly instead of dynamic type detection

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dlevy-msft-sql dlevy-msft-sql added bug Something isn't working pr-size: medium Moderate update size labels Dec 1, 2025
@github-actions
Copy link

github-actions bot commented Jan 5, 2026

📊 Code Coverage Report

🔥 Diff Coverage

87%


🎯 Overall Coverage

76%


📈 Total Lines Covered: 5524 out of 7196
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/init.py (100%)
  • mssql_python/connection.py (76.2%): Missing lines 986,991,1008-1009,1020
  • mssql_python/constants.py (100%)
  • mssql_python/cursor.py (85.7%): Missing lines 1019,1361,2429
  • mssql_python/type_code.py (93.9%): Missing lines 91,115

Summary

  • Total: 79 lines
  • Missing: 10 lines
  • Coverage: 87%

mssql_python/connection.py

Lines 982-995


Lines 1004-1013


Lines 1016-1024


mssql_python/cursor.py

Lines 1015-1023


Lines 1357-1365


Lines 2425-2433


mssql_python/type_code.py

Lines 87-95


Lines 111-116



📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.cpp: 69.3%
mssql_python.pybind.ddbc_bindings.h: 69.7%
mssql_python.pybind.connection.connection.cpp: 75.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 83.4%
mssql_python.cursor.py: 84.9%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@dlevy-msft-sql dlevy-msft-sql added pr-size: small Minimal code update pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Jan 16, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dlevy-msft-sql dlevy-msft-sql linked an issue Jan 19, 2026 that may be closed by this pull request
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dlevy-msft-sql added a commit to dlevy-msft-sql/mssql-python that referenced this pull request Jan 20, 2026
- Add SQL_SS_XML (-152) constant to Python constants.py
  (was incorrectly using SQL_XML = 241)
- Add SQL_SS_TIME2 (-154) constant for SQL Server TIME(n) type
- Update cursor type map to use SQL_SS_XML and add SQL_SS_TIME2 mapping
- Add sync comment in C++ to prevent future constant drift

Constants verified against Microsoft Learn ODBC documentation:
- SQL_SS_TIME2: -154 (SQLNCLI.h)
- SQL_SS_TIMESTAMPOFFSET: -155 (SQLNCLI.h)
- SQL_SS_XML: -152 (SQL Server ODBC driver)
- SQL_SS_UDT: -151 (SQL Server ODBC driver)

Addresses Copilot review feedback on PR microsoft#355
@dlevy-msft-sql dlevy-msft-sql changed the title FIX: cursor.description returns dual-compatible SqlTypeCode type codes FIX: Cursor.description returns dual-compatible SqlTypeCode type codes Feb 7, 2026
@dlevy-msft-sql dlevy-msft-sql force-pushed the Issue-352 branch 4 times, most recently from f2d2c33 to e734a2f Compare February 7, 2026 22:03
@dlevy-msft-sql dlevy-msft-sql marked this pull request as ready for review February 8, 2026 01:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dlevy-msft-sql
Copy link
Contributor Author

Addressed 3 of 5 comments, dismissed 2 with explanations:

Fixed:

  1. SQL_SS_UDT #ifndef guard (ddbc_bindings.cpp) — Added #ifndef SQL_SS_UDT / #endif to match the pattern used for SQL_DATETIME2 and SQL_SMALLDATETIME.
  2. _build_converter_map returning None (cursor.py) — Removed the if not any(converter_map): return None check. Now always returns the list, so Row takes the optimized _apply_output_converters_optimized path (which correctly handles None converters per-cell) instead of falling back to the slower legacy per-row lookup.
  3. remove_output_converter asymmetry (connection.py) — Now also removes the python_type key when a SqlTypeCode is passed, symmetric with get_output_converter's fallback lookup.

Dismissed:
4. __eq__ reverse comparison / int subclass — Python's reflected comparison protocol already handles this. When int.__eq__(SqlTypeCode_instance) returns NotImplemented, Python falls back to SqlTypeCode.__eq__(int_value) which returns True. Both SqlTypeCode(-9) == -9 and -9 == SqlTypeCode(-9) work, as does type_code in (SQL_WVARCHAR, SQL_VARCHAR) (tuple in uses == which triggers the reflected path). Making SqlTypeCode an int subclass would create new problems — int.__eq__ would handle int comparisons directly but type.__eq__ on the LHS type object would still return False for str == SqlTypeCode(-9), requiring the same reflected path.
5. __hash__ = None vs custom raise TypeError — Our custom error message gives significantly better DX: "SqlTypeCode is unhashable. Use int(type_code) or type_code.type_code as a dict key instead. Example: {int(desc[1]): handler}" vs the generic "unhashable type: 'SqlTypeCode'". The .pyi stub already declares __hash__: None for type checker compatibility.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

mssql_python/mssql_python.pyi:169

  • In the type stub, Row’s description currently types type_code as Union[SqlTypeCode, type], but the implementation now always populates cursor.description[i][1] with a SqlTypeCode instance (which is comparable to type/int). Updating the stub to SqlTypeCode (or a protocol capturing __int__ + comparisons) will better match runtime behavior and avoid type-checker false positives (e.g., int(desc[i][1])).
        description: List[
            Tuple[
                str,
                Union[SqlTypeCode, type],
                Optional[int],
                Optional[int],
                Optional[int],
                Optional[int],
                Optional[bool],
            ]

mssql_python/mssql_python.pyi:205

  • Cursor.description is documented here as returning SqlTypeCode, but the stub still uses Union[SqlTypeCode, type] for the type_code element. Since cursor._initialize_description() now always wraps the raw SQL code as SqlTypeCode(...), the stub should reflect SqlTypeCode as the return type to align with runtime and prevent incorrect static typing downstream.
    # description is a sequence of 7-item tuples:
    # (name, type_code, display_size, internal_size, precision, scale, null_ok)
    # type_code is SqlTypeCode which compares equal to both SQL integers and Python types
    description: Optional[
        List[
            Tuple[
                str,
                Union[SqlTypeCode, type],
                Optional[int],
                Optional[int],
                Optional[int],
                Optional[int],
                Optional[bool],

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dlevy-msft-sql dlevy-msft-sql marked this pull request as draft February 8, 2026 03:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dlevy-msft-sql dlevy-msft-sql marked this pull request as ready for review February 8, 2026 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cursor.description returning incorrect values

3 participants