FIX: Align date/time type code mappings with ODBC 18 driver source (#352)#355
FIX: Align date/time type code mappings with ODBC 18 driver source (#352)#355dlevy-msft-sql wants to merge 1 commit intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
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.
3f52dd7 to
646ef04
Compare
📊 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.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: 84.1%
mssql_python.cursor.py: 84.7%🔗 Quick Links
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
- 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
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3bf27c3 to
0d52ef5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0d52ef5 to
21705fe
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
21705fe to
f70e7e0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f70e7e0 to
ff971e0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ff971e0 to
298af26
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Root cause: _map_data_type only had ODBC 2.x constants (SQL_DATE=9, SQL_TIME=10, SQL_TIMESTAMP=11) but the ODBC 18 driver reports ODBC 3.x codes via SQLDescribeCol. Date columns returned SQL_TYPE_DATE(91) which fell through to str default, causing polars ComputeError. Verified against ODBC 18 driver source (sqlcmisc.cpp rgbSRV2SQLTYPE[] and sqlcdesc.cpp SQL_DESC_CONCISE_TYPE handling): constants.py: - Add SQL_SS_TIME2(-154), SQL_SS_XML(-152), SQL_C_SS_TIME2(0x4000) cursor.py _map_data_type: - Replace ODBC 2.x entries with driver-verified ODBC 3.x codes - SQL_TYPE_DATE(91) -> datetime.date - SQL_TYPE_TIMESTAMP(93) -> datetime.datetime - SQL_SS_TIME2(-154) -> datetime.time - SQL_DATETIMEOFFSET(-155) -> datetime.datetime - SQL_SS_XML(-152) -> str - Add missing types: SQL_LONGVARCHAR, SQL_WLONGVARCHAR, SQL_REAL cursor.py _get_c_type_for_sql_type: - SQL_TYPE_DATE -> SQL_C_TYPE_DATE (was SQL_DATE) - SQL_SS_TIME2 -> SQL_C_SS_TIME2 (was SQL_TIME) - SQL_TYPE_TIMESTAMP -> SQL_C_TYPE_TIMESTAMP (was SQL_TIMESTAMP) - Add SQL_DATETIMEOFFSET -> SQL_C_SS_TIMESTAMPOFFSET Tests: - 14 tests: cursor.description type_code verification (6 date/time types + isclass check), polars integration (4), pandas integration (3) Closes microsoft#352
298af26 to
ea8362b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 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.
I had the diagnosis wrong and went a long way down the wrong road. The fix was actually simple. It should work without any code change for you now. |
Work Item / Issue Reference
Summary
Fixes
polars.read_database()ComputeErroron DATE columns by aligningcursor.pytype code mappings with the ODBC 18 driver's actual reported SQL type codes.Root Cause:
The ODBC 18 driver reports ODBC 3.x type codes (
SQL_TYPE_DATE=91,SQL_TYPE_TIMESTAMP=93,SQL_SS_TIME2=-154) but_map_data_typeonly had ODBC 2.x constants (SQL_DATE=9,SQL_TIME=10,SQL_TIMESTAMP=11). DATE columns fell through to thestrdefault, causing a schema mismatch when polars expectedpl.Date.Fix (verified against ODBC 18 driver C++ source):
_map_data_type: Rewritten with driver-verified ODBC 3.x type codes organized by category (string, integer, float, decimal, date/time, boolean, binary, UUID, XML). Removed phantom ODBC 2.x entries that the driver never reports._get_c_type_for_sql_type: Updated C-type bindings from ODBC 2.x to 3.x codes for all date/time types.constants.py: AddedSQL_SS_TIME2,SQL_SS_XML,SQL_C_SS_TIME2.Zero breaking changes.
cursor.description[i][1]continues to return Python type objects (datetime.date,datetime.datetime, etc.) — now mapped from the correct SQL type codes.Changes
mssql_python/constants.pySQL_SS_TIME2,SQL_SS_XML,SQL_C_SS_TIME2)mssql_python/cursor.py_map_data_typeand_get_c_type_for_sql_typewith ODBC 3.x codestests/test_018_polars_pandas_integration.pyTesting
14 new tests across 3 test classes:
TestCursorDescriptionTypeCodes(7 tests): Verifies all 6 date/time SQL types return correct Python types withisclass()checksTestPolarsIntegration(4 tests):polars.read_database()with DATE, all datetime types, mixed types, NULLsTestPandasIntegration(3 tests):pandas.read_sql()with DATE, all datetime types, mixed typesAll 14 tests pass. Existing test suite (448 tests) unaffected.