FIX: executemany SQL_C_NUMERIC mismatch#611
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes a failure in Cursor.executemany() when binding large Decimal values (outside SQL Server MONEY range) by ensuring the detected parameter C type matches the actual converted parameter data, and improves bulkcopy() compatibility with iterable row objects (e.g., Row from fetchmany). It also adds unit and integration tests covering the reported GH-609 scenario.
Changes:
- Adjust
executemanyauto-detected parameter binding forDECIMAL/NUMERICto useSQL_C_CHARto match the Decimal-to-string conversion path (GH-609). - Add a
bulkcopyadapter to convert iterable row objects to tuples before passing to the Rust backend. - Add new unit + integration tests covering large Decimal batches (including mixed batches and NULLs).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
mssql_python/cursor.py |
Updates executemany DECIMAL/NUMERIC binding and adds tuple-normalization for bulkcopy input rows. |
tests/test_001_globals.py |
Adds unit tests for the executemany Decimal type-detection/override pipeline (GH-609). |
tests/test_004_cursor.py |
Adds integration tests reproducing and validating the GH-609 executemany scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7d46896 to
73ab7a2
Compare
…H-609) Bug 1: executemany auto-detection path set paramCType=SQL_C_NUMERIC for Decimal values outside the MONEY range, but the conversion loop converts all Decimals to strings. The C extension then received strings where it expected NumericData structs, raising RuntimeError. Fixed by overriding SQL_C_NUMERIC to SQL_C_CHAR in the auto-detection path, matching the existing GH-503 fix in the setinputsizes path. Bug 2: bulkcopy passed Row objects directly to mssql_py_core which expects native tuples. Added _ensure_tuples() wrapper to auto-convert Row/list objects to tuples. Fixes #609
4b6fb85 to
d454a6f
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.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.7%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.1%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.pybind.connection.connection.cpp: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%🔗 Quick Links
|
…EY range (GH-609) The executemany auto-detection path set paramCType=SQL_C_NUMERIC for Decimal values outside the MONEY range, but the conversion loop converts all Decimals to strings. The C extension then received strings where it expected NumericData structs, raising RuntimeError. Fixed by overriding SQL_C_NUMERIC to SQL_C_CHAR in the auto-detection path, matching the existing GH-503 fix in the setinputsizes path. Fixes #609
d454a6f to
ba19ba7
Compare
bewithgaurav
left a comment
There was a problem hiding this comment.
reproduced #609 against sql2022, fix is clear and well-scoped. lgtm.
… jahnvi/github_issue_609
4de7b2b to
188fef5
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
### Work Item / Issue Reference <!-- IMPORTANT: Please follow the PR template guidelines below. For mssql-python maintainers: Insert your ADO Work Item ID below For external contributors: Insert Github Issue number below Only one reference is required - either GitHub issue OR ADO Work Item. --> <!-- mssql-python maintainers: ADO Work Item --> > [AB#45380](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/45380) <!-- External contributors: GitHub Issue --> > GitHub Issue: #609 ------------------------------------------------------------------- ### Summary This pull request addresses a critical bug in the handling of `executemany` with large `Decimal` values in the `mssql_python` driver, specifically when values exceed the SQL Server `MONEY` range. The main fix ensures that parameter type detection and conversion are consistent, preventing runtime errors when binding large decimal values. Extensive unit and integration tests are added to verify the fix and cover edge cases involving `Decimal` values, including scenarios with `NULL`s and multi-column inserts. **Bug Fix: Executemany Decimal Handling** * In `cursor.py`, the `executemany` method now explicitly overrides the C type for parameters with SQL type `DECIMAL` or `NUMERIC` to use `SQL_C_CHAR` (string binding) when the data is converted to strings. This prevents mismatches that previously caused runtime errors when inserting large decimal values. The column size is also adjusted to fit the longest string representation. **Testing: Unit and Integration Tests for Decimal Handling** * Added comprehensive unit tests in `test_001_globals.py` to verify type detection, mapping, and the override logic for `executemany` with large `Decimal` values, both within and outside the `MONEY` range. These tests confirm that the C type override is necessary and correctly applied. * Added integration tests in `test_004_cursor.py` to exercise the fixed behavior in real database scenarios, including: - Inserting batches with decimals inside and outside the `MONEY` range. - Handling `NULL` values alongside large decimals. - Multi-column inserts where one column contains large decimals.
Work Item / Issue Reference
Summary
This pull request addresses a critical bug in the handling of
executemanywith largeDecimalvalues in themssql_pythondriver, specifically when values exceed the SQL ServerMONEYrange. The main fix ensures that parameter type detection and conversion are consistent, preventing runtime errors when binding large decimal values. Extensive unit and integration tests are added to verify the fix and cover edge cases involvingDecimalvalues, including scenarios withNULLs and multi-column inserts.Bug Fix: Executemany Decimal Handling
cursor.py, theexecutemanymethod now explicitly overrides the C type for parameters with SQL typeDECIMALorNUMERICto useSQL_C_CHAR(string binding) when the data is converted to strings. This prevents mismatches that previously caused runtime errors when inserting large decimal values. The column size is also adjusted to fit the longest string representation.Testing: Unit and Integration Tests for Decimal Handling
test_001_globals.pyto verify type detection, mapping, and the override logic forexecutemanywith largeDecimalvalues, both within and outside theMONEYrange. These tests confirm that the C type override is necessary and correctly applied.test_004_cursor.pyto exercise the fixed behavior in real database scenarios, including:MONEYrange.NULLvalues alongside large decimals.