FIX: Use ODBC 3.x parameter types instead of ODBC 2.x - #758
Conversation
📊 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: 57.9%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 77.5%
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
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes directly satisfy the stated acceptance criteria and are backed by focused tests covering both inference and setinputsizes() canonicalization.
Pull request overview
This PR updates the Python-side parameter type inference and setinputsizes() handling to consistently use ODBC 3.x concise temporal SQL type identifiers (DATE/TIME/TIMESTAMP) when binding parameters, aligning the driver’s behavior with its declared ODBC 3.8 application mode and improving compatibility with ODBC 3.x–only drivers (per #757).
Changes:
- Switch Python-side inference for
datetime.date/ naivedatetime.datetimetoSQL_TYPE_DATE/SQL_TYPE_TIMESTAMP. - Canonicalize legacy ODBC 2.x temporal hints provided via
Cursor.setinputsizes()to their ODBC 3.x equivalents before storage/binding. - Add focused unit coverage for the updated inference and canonicalization behavior, and update execute-path parity expectations.
File summaries
| File | Description |
|---|---|
mssql_python/cursor.py |
Updates temporal inference to ODBC 3.x types and canonicalizes legacy temporal types in setinputsizes(). |
tests/test_004_cursor.py |
Adds targeted tests asserting ODBC 3.x temporal inference and setinputsizes() canonicalization. |
tests/test_023_execute_path_parity.py |
Aligns parity test expectations to ODBC 3.x temporal SQL type constants. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Gaurav Sharma (bewithgaurav)
left a comment
Gaurav Sharma (bewithgaurav)
left a comment
There was a problem hiding this comment.
lgtm
…poral-types # Conflicts: # mssql_python/cursor.py
There was a problem hiding this comment.
🟢 Approval recommended
The changes directly meet the issue’s acceptance criteria and include focused tests verifying both inference and setinputsizes() canonicalization for temporal types.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Resolve conflict in execute(): keep upstream native DDBCSQLExecute / DDBCSQLExecDirect path from microsoft#736/microsoft#758; retain microsoftGH-745 executemany SQL_NUMERIC batch precision/scale changes.
Work Item / Issue Reference
Summary
Use ODBC 3.x concise temporal SQL types for Python-side parameter inference. Canonicalize legacy temporal hints passed to
setinputsizes()before binding while preserving exported constant values. Add focused coverage for inferred and explicit temporal types.Validation
python -m black --check --line-length=100 mssql_python/ tests/tests/test_000_dependencies.py: 35 passed, 3 skippedDB_CONNECTION_STRINGis unavailable.