FIX: bind Decimal as SQL_NUMERIC regardless of value by bewithgaurav · Pull Request #742 · microsoft/mssql-python

Gaurav Sharma (bewithgaurav) · 2026-09-02T09:49:04Z

Work Item / Issue Reference

GitHub Issue: #740

Summary

The standard execute path chose a Decimal's bind type from its value, sending
anything in the MONEY/SMALLMONEY range as a formatted VARCHAR. Comparing such a
value against a smaller numeric column made SQL Server convert varchar to numeric
and overflow, so WHERE v = ? raised an arithmetic overflow instead of simply not
matching. Bind every finite Decimal as SQL_NUMERIC with its own precision and
scale, matching pyodbc.

Removing the shortcut surfaced a second bug: the numeric parameter's descriptor
record number was hardcoded to 1, so a numeric parameter in any position other than
the first wrote its precision/scale onto the wrong record and the driver raised
"Numeric value out of range". Use the parameter's own 1-based position.

The fix covers both execute() routes: the native C++ detection path, and the Python
legacy path reached when setinputsizes() covers fewer positions than parameters (an
uncovered money-range Decimal there previously fell back to the VARCHAR shortcut and
overflowed). executemany is intentionally left on its batch VARCHAR string binding
(GH-503), so a money-range Decimal compared through executemany can still overflow;
that is a tangential finding filed separately as #745.

The standard execute path chose a Decimal's bind type from its value: anything in the MONEY/SMALLMONEY range was sent as a formatted VARCHAR. Comparing such a value against a smaller numeric column made SQL Server convert varchar to numeric and overflow, so 'WHERE v = ?' raised an arithmetic overflow instead of just not matching. Bind every finite Decimal as SQL_NUMERIC with its own precision and scale, matching pyodbc. Removing the shortcut surfaced a second bug: the numeric parameter's APD record number in SQLSetDescField was hardcoded to 1, so a numeric parameter in any position other than the first wrote its precision/scale onto the wrong record and the driver raised 'Numeric value out of range'. Use the parameter's own 1-based position. Scoped to the single execute() path; executemany still string-binds decimals (GH-503) and is a separate follow-up. (GH-740) Co-authored-by: Copilot <[email protected]>

Copilot

🔵 Needs a closer look

It changes native ODBC parameter binding behavior (descriptor manipulation and Decimal typing), which warrants final human review despite good regression coverage.

Pull request overview

This PR adjusts the driver’s execute() fast path parameter detection/binding so Python Decimal values are always bound as SQL_NUMERIC (with derived precision/scale), avoiding SQL Server’s server-side varchar→numeric conversion overflow behavior seen when Decimals were previously string-bound in MONEY/SMALLMONEY ranges, and fixes descriptor-record selection for non-first numeric parameters.

Changes:

Bind all finite Decimal parameters as SQL_NUMERIC in DetectParamTypes (removing the MONEY/SMALLMONEY VARCHAR shortcut).
Fix numeric APD descriptor record selection to use the parameter’s own 1-based position instead of hardcoding record 1.
Add regression tests covering GH-740 scenarios (overflow avoidance, non-first-position numeric params, multiple numerics, boundary round-trips, re-exec with changing precision/scale).

File summaries

File	Description
tests/test_020_money_smallmoney.py	Updates module-level behavior description and adds GH-740 regression tests for Decimal numeric binding and descriptor record handling.
mssql_python/pybind/py_type_cache.hpp	Removes cached MONEY/SMALLMONEY boundary Decimal objects no longer needed after eliminating range-based binding.
mssql_python/pybind/param_detect.hpp	Removes MONEY/SMALLMONEY range detection/string-binding; always constructs `NumericData` and sets `SQL_NUMERIC` binding for finite Decimals.
mssql_python/pybind/ddbc_bindings.cpp	Fixes numeric APD descriptor record number to match the actual 1-based parameter index when setting precision/scale/data ptr.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Clarify that the always-SQL_NUMERIC binding applies to execute(); executemany still string-binds Decimals (GH-503). (GH-740) Co-authored-by: Copilot <[email protected]>

github-actions · 2026-09-02T10:07:07Z

📊 Code Coverage Report

🔥 Diff Coverage 60%	🎯 Overall Coverage 82%	📈 Total Lines Covered: `7772` out of `9444` 📁 Project: `mssql-python`

Diff Coverage

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

mssql_python/cursor.py (100%)
mssql_python/pybind/ddbc_bindings.cpp (42.9%): Missing lines 879-881,919

Summary

Total: 10 lines
Missing: 4 lines
Coverage: 60%

mssql_python/pybind/ddbc_bindings.cpp

Lines 875-885

  875             // The APD record number is the 1-based parameter position, matching the
  876             // SQLBindParameter call above. It was previously hardcoded to 1, so a
  877             // SQL_C_NUMERIC parameter in any position other than the first had its
  878             // precision/scale/data pointer written onto record 1 instead of its own.
! 879             // The driver then read the numeric struct with the wrong descriptor and
! 880             // raised "Numeric value out of range" (GH-740).
! 881             const SQLSMALLINT descRecNum = static_cast<SQLSMALLINT>(paramIndex + 1);
  882             SQLHDESC hDesc = nullptr;
  883             rc = SQLGetStmtAttr_ptr(hStmt, SQL_ATTR_APP_PARAM_DESC, &hDesc, 0, NULL);
  884             if (!SQL_SUCCEEDED(rc)) {
  885                 LOG("BindParameters: SQLGetStmtAttr(SQL_ATTR_APP_PARAM_DESC) "

Lines 915-923

  915                     paramIndex, rc);
  916                 return rc;
  917             }
  918 
! 919             rc = SQLSetDescField_ptr(hDesc, descRecNum, SQL_DESC_DATA_PTR,
  920                                      reinterpret_cast<SQLPOINTER>(numericPtr), 0);
  921             if (!SQL_SUCCEEDED(rc)) {
  922                 LOG("BindParameters: SQLSetDescField(SQL_DESC_DATA_PTR) failed "
  923                     "for param[%d] - SQLRETURN=%d",

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.cpp: 58.9%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 75.6%
mssql_python.__init__.py: 77.6%
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.connection.py: 85.9%

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

test_money_range_decimal_binds_wide only round-tripped the value, so it stayed green after the native C type changed to NUMERIC. Assert the declared base type via sql_variant, and note that _map_sql_type still text-binds money-range Decimals to protect executemany's string binding. (GH-740) Co-authored-by: Copilot <[email protected]>

The position test used a NULL first param, which masked the old record-1 bug (record 1 held no data). Use a non-null value first and assert it round-trips intact, so the test pins the collateral corruption of the earlier parameter, not just the numeric's own misplacement. Verified it fails against the pre-fix binder. (GH-740) Co-authored-by: Copilot <[email protected]>

The native execute() path was fixed for GH-740, but a real execute() still reaches the Python _map_sql_type when setinputsizes() covers fewer positions than parameters: the uncovered money-range Decimal took the VARCHAR shortcut and overflowed a numeric comparison. Thread a decimal_as_numeric flag so the legacy execute path binds every finite Decimal as SQL_NUMERIC, while executemany keeps its batch VARCHAR string binding (GH-503) unchanged. Adds a partial-setinputsizes regression test. Co-authored-by: Copilot <[email protected]>

…//github.com/microsoft/mssql-python into bewithgaurav/fix-740-decimal-numeric-binding

Jahnvi Thakkar (jahnvi480) · 2026-09-03T08:25:39Z

This is a behavioral change on the wire (money-range Decimals now bind NUMERIC, not VARCHAR) - bare SELECT ? returns Decimal not str, sql_variant stores numeric, and since NUMERIC outranks MONEY/VARCHAR in type precedence, WHERE money_or_varchar_col = ? can flip an index seek to a CONVERT_IMPLICIT scan. Worth a CHANGELOG entry.

Sumit Sarabhai (sumitmsft)

Reviewed the PR for correctness, security, reliability, performance, test coverage, repository conventions, and applicable architecture and design specifications. No actionable issues were identified. The implementation is consistent with repository standards and the applicable approved design requirements.

The earlier finding on the legacy execute path (an uncovered money-range Decimal binding as VARCHAR when setinputsizes is shorter than the parameter list) is fully addressed here via decimal_as_numeric=True, with a dedicated regression test in tests/test_023_execute_path_parity.py. The executemany residual is disclosed in the description and tracked as a separate follow-up (#745).

Recommendation: Approve

Document the money-range Decimal binding change (now numeric, not varchar) in CHANGELOG, including the on-the-wire behavioral effects (SELECT ? returns Decimal, sql_variant base type, and numeric type-precedence causing CONVERT_IMPLICIT on the column side). Add two edge-case tests: an exact numeric(38,38) round-trip of 1E-38 asserted via as_tuple() (loose-tolerance coverage elsewhere would miss a silent zero), and signed-zero normalization. Co-authored-by: Copilot <[email protected]>

…//github.com/microsoft/mssql-python into bewithgaurav/fix-740-decimal-numeric-binding

Reconcile #742 (bind Decimal as SQL_NUMERIC regardless of value, GH-740) with the native setinputsizes migration: - param_detect.hpp: drop the automatic MONEY/SMALLMONEY VARCHAR shortcut; every finite Decimal binds SQL_NUMERIC natively. FormatDecimalParam stays for the setinputsizes DECIMAL override only. - cursor.py: _create_parameter_types_list forwards decimal_as_numeric to _map_sql_type; the parameterless else-branch keeps DDBCSQLExecDirect and drops the deleted DDBCSQLExecuteLegacy block (GH-740 fix now happens in native detection). - test_023: keep both suites; narrow test_decimal_format_must_return_string to the setinputsizes override, the only path that still formats Decimals after GH-740. Co-authored-by: Copilot <[email protected]>

Point the Windows PR-validation legs at the mssql-python wheel from Build-Release-Package-Pipeline dev build 172271, which already carries the two pyodbc-parity fixes (microsoft/mssql-python#741 Binary(memoryview) and microsoft/mssql-python#742 Decimal SQL_NUMERIC) with the native core rebuilt. The published PyPI 1.14.0 wheel does not have these yet, so the two Decimal and BinaryField gaps would otherwise still fail. - tox.ini: allow the mssql-python requirement to be overridden by MSSQL_PYTHON_WHEEL, defaulting to the PyPI requirement when unset. - azure-pipelines-steps-windows.yml: download the matching per-Python wheel from build 172271 and hand its path to tox. - azure-pipelines.yml: gate Linux_Core, Linux_Legacy and Windows_Legacy off for this run. Build 172271 produced no Linux wheels, and the EOL/py3.8-3.9 legs have no wheel, so only the supported Windows matrix is validated here. Co-authored-by: Copilot <[email protected]>

Cross-project artifact download from the mssql-python project is blocked for the public project's build identity (VS800075), so commit the Windows wheels directly under ci/mssql-python-wheels/ and install the per-Python wheel from there. Wheels are from Build-Release-Package-Pipeline dev build 172271 and carry microsoft/mssql-python#741 (Binary(memoryview)) and microsoft/mssql-python#742 (Decimal SQL_NUMERIC). Temporary: drop once mssql-python 1.15.0 ships to PyPI with both fixes. Co-authored-by: Copilot <[email protected]>

…#742) on merge #720 branched before #742 landed the money-range Decimal->SQL_NUMERIC fix (the decimal_as_numeric path), so a careless 'take ours' merge would have silently DELETED 29 lines of that shipped correctness logic -- invisible in the PR's own diff. Restored main's cursor.py (which has #742) and re-applied ONLY #720's intended change: the bulkcopy ImportError reword (GH-619, names the Windows-ARM64 / not-shipped-on-every-platform case). git diff origin/main HEAD -- mssql_python/cursor.py now shows only that one hunk. Black + compile clean.

…n/main Bulk copy IS now available on Windows ARM64 (the mssql_py_core arm64 core was built + shipped and validated in production pipelines), so the reword naming win-arm64 as an 'not shipped on every platform' example is factually stale. cursor.py is also out of scope for this conda-pipeline PR. Reverted it to origin/main exactly -- which KEEPS the #742 (GH-740) money-range Decimal->SQL_NUMERIC fix (decimal_as_numeric, 9 refs present) and removes the stale message. git diff origin/main HEAD -- mssql_python/cursor.py is now EMPTY, so #720 no longer touches cursor.py (no merge-revert risk, no scope creep). Any improved bulkcopy-unavailable message belongs in a separate GH-619 product PR with accurate current platform coverage.

The full Build-Release-Package-Pipeline run 173176 (main, commit b426da4e) produced Linux wheels as well, so extend the wheel validation to the Linux supported matrix: - Refresh ci/mssql-python-wheels/ with the Windows and Linux x86_64 wheels from a single build (173176). Both still carry microsoft/mssql-python#741 (Binary(memoryview)) and microsoft/mssql-python#742 (Decimal SQL_NUMERIC). - azure-pipelines-steps-linux.yml: install the per-Python manylinux wheel from ci/mssql-python-wheels/ via MSSQL_PYTHON_WHEEL, mirroring the Windows steps. - azure-pipelines.yml: re-enable Linux_Core and raise its timeout to 180 min to match Windows_Core (mssql-python runs the suite ~1.5-2x slower). Windows_Legacy and Linux_Legacy stay gated off (no wheels for EOL py3.8/3.9). Temporary: drop once mssql-python 1.15.0 ships to PyPI with both fixes. Co-authored-by: Copilot <[email protected]>

[AB#48121](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/48121) ### Summary #### Enhancements - Add selectable native ODBC providers (#730). - Route `setinputsizes()` through the native C++ execution pipeline (#736). - Accept `memoryview` values in `Binary()` (#741). - Expose SQL Server type constants at module level (#764). #### Bug Fixes - Prevent concurrent logging deadlocks (#678). - Vendor the correct `mssql_py_core` architecture in Windows ARM64 wheels (#737). - Resolve bundled Windows driver and authentication DLLs from package-local directories (#735). - Bind `Decimal` parameters consistently as `SQL_NUMERIC` (#742). - Use ODBC 3.x parameter types (#758). - Decode `SQL_DATABASE_NAME` metadata (#771). - Prevent shutdown crashes during mixed cursor cleanup (#772). Bumps the package version from 1.14.0 to 1.15.0 and refreshes the PyPI release summary. --------- Co-authored-by: Gaurav Sharma <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]>

* FEAT: [mssql-python] Integrate mssql-python into packaging and CI Wire the opt-in mssql-python driver into packaging and the test matrix: - setup.py: add an optional 'mssql-python' extra pinned to mssql-python>=1.15.0. pyodbc stays in install_requires, so the default install is unchanged and mssql-python is pulled in only via 'pip install mssql-django[mssql-python]'. - testapp/settings.py: source the python_driver and extra_params OPTIONS from the MSSQL_PYTHON_DRIVER / MSSQL_EXTRA_PARAMS environment variables (defaulting to empty), so the test app selects the driver without code changes. - tox.ini: on Python 3.10+ envs (mssql-python's supported floor) install mssql-python>=1.15.0 and set MSSQL_PYTHON_DRIVER=mssql_python plus MSSQL_EXTRA_PARAMS=TrustServerCertificate=yes so those legs exercise the mssql-python path end to end. The legacy (py36-py39) envs are untouched and continue to run under pyodbc. This is the final integration task of the mssql-django 2.0 opt-in mssql-python work. It intentionally depends on mssql-python 1.15.0, which publishes on 11 September 2026; until then the Python 3.10+ legs are expected to be red, turning green once 1.15.0 is available. The end-to-end path was validated green across the supported matrix using the fixes shipping in 1.15.0 (microsoft/mssql-python#741, microsoft/mssql-python#742). AB# Co-authored-by: Copilot <[email protected]> * CHORE: Modernize coverage for supported Python versions AB#48003 Co-authored-by: Copilot <[email protected]> * FIX: Validate selected CI driver identities AB#48003 Co-authored-by: Copilot <[email protected]> * FIX: Scope native driver validation to CI Co-authored-by: Copilot <[email protected]> * FEAT: Require mssql-python for mssql-django 2.0 Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>

Copilot AI lite review requested due to automatic review settings September 2, 2026 09:49

github-actions Bot added the pr-size: medium Moderate update size label Sep 2, 2026

Copilot started reviewing on behalf of Gaurav Sharma (bewithgaurav) September 2, 2026 09:49 View session

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Comment thread tests/test_020_money_smallmoney.py Outdated

DOC: scope test_020 docstring to the execute() path

bb77e1e

Clarify that the always-SQL_NUMERIC binding applies to execute(); executemany still string-binds Decimals (GH-503). (GH-740) Co-authored-by: Copilot <[email protected]>

Jahnvi Thakkar (jahnvi480) reviewed Sep 2, 2026

View reviewed changes

Comment thread mssql_python/pybind/param_detect.hpp

Jahnvi Thakkar (jahnvi480) reviewed Sep 2, 2026

View reviewed changes

Comment thread tests/test_020_money_smallmoney.py

Jahnvi Thakkar (jahnvi480) reviewed Sep 2, 2026

View reviewed changes

Comment thread tests/test_020_money_smallmoney.py

Copilot AI and others added 3 commits September 2, 2026 21:18

Merge branch 'main' into bewithgaurav/fix-740-decimal-numeric-binding

2a93752

Sumit Sarabhai (sumitmsft) reviewed Sep 2, 2026

View reviewed changes

Comment thread mssql_python/pybind/param_detect.hpp

Copilot AI and others added 3 commits September 3, 2026 12:07

Merge branch 'bewithgaurav/fix-740-decimal-numeric-binding' of https:…

ffe936e

…//github.com/microsoft/mssql-python into bewithgaurav/fix-740-decimal-numeric-binding

Merge branch 'main' into bewithgaurav/fix-740-decimal-numeric-binding

1fcda2c

Jahnvi Thakkar (jahnvi480) reviewed Sep 3, 2026

View reviewed changes

Comment thread tests/test_020_money_smallmoney.py

Sumit Sarabhai (sumitmsft) previously approved these changes Sep 3, 2026

View reviewed changes

Copilot AI and others added 2 commits September 3, 2026 14:32

Merge branch 'bewithgaurav/fix-740-decimal-numeric-binding' of https:…

4cafa50

…//github.com/microsoft/mssql-python into bewithgaurav/fix-740-decimal-numeric-binding

Gaurav Sharma (bewithgaurav) dismissed Sumit Sarabhai (sumitmsft)’s stale review via 4cafa50 September 3, 2026 09:03

Jahnvi Thakkar (jahnvi480) approved these changes Sep 3, 2026

View reviewed changes

Sumit Sarabhai (sumitmsft) approved these changes Sep 3, 2026

View reviewed changes

Gaurav Sharma (bewithgaurav) merged commit d683962 into main Sep 3, 2026
29 checks passed

VyrnSynx (vyrnsynx) mentioned this pull request Sep 4, 2026

FIX: bind executemany money-range Decimals as SQL_NUMERIC #752

Open

Subrata (subrata-ms) mentioned this pull request Sep 11, 2026

RELEASE:1.15.0 #782

Merged

Michael Bayer (zzzeek) mentioned this pull request Sep 11, 2026

mssql+mssqlpython raises arithmetic overflow for Decimal values with mssql-python 1.15 sqlalchemy/sqlalchemy#13585

Closed

Navigation Menu

Uh oh!

FIX: bind Decimal as SQL_NUMERIC regardless of value - #742

FIX: bind Decimal as SQL_NUMERIC regardless of value#742
Gaurav Sharma (bewithgaurav) merged 10 commits into
mainfrom
bewithgaurav/fix-740-decimal-numeric-binding

Gaurav Sharma (bewithgaurav) commented Sep 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 2, 2026 •

edited

Loading

🔥 Diff Coverage

60%

🎯 Overall Coverage

82%

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jahnvi Thakkar (jahnvi480) commented Sep 3, 2026

Uh oh!

Uh oh!

Sumit Sarabhai (sumitmsft) left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

Gaurav Sharma (bewithgaurav) commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Work Item / Issue Reference

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

github-actions Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Code Coverage Report

🔥 Diff Coverage

60%

🎯 Overall Coverage

82%

Diff Coverage

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

Summary

mssql_python/pybind/ddbc_bindings.cpp

📋 Files Needing Attention

🔗 Quick Links

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jahnvi Thakkar (jahnvi480) commented Sep 3, 2026

Uh oh!

Uh oh!

Sumit Sarabhai (sumitmsft) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Gaurav Sharma (bewithgaurav) commented Sep 2, 2026 •

edited

Loading

github-actions Bot commented Sep 2, 2026 •

edited

Loading