FIX: Setinputsizes() SQL_DECIMAL crash

[jahnvi480]

Work Item / Issue Reference

AB#53990

GitHub Issue: #503

---

Summary

This pull request improves the handling of Python Decimal values when binding to SQL DECIMAL and NUMERIC types, especially when using setinputsizes and executemany. It fixes a runtime error by ensuring Decimal objects are converted to strings for proper binding, and adds comprehensive tests to verify this behavior.

Decimal binding and conversion improvements:

• Changed the mapping for SQL DECIMAL and NUMERIC types to use SQL_C_CHAR instead of SQL_C_NUMERIC in _get_c_type_for_sql_type, enabling string-based binding for decimals.
• Updated parameter handling in _create_parameter_types_list and executemany to convert Python Decimal objects to strings when binding to SQL DECIMAL or NUMERIC columns.

Testing enhancements:

• Added new tests in test_004_cursor.py to verify that setinputsizes with SQL_DECIMAL and SQL_NUMERIC accepts Python Decimal values, works with both executemany and execute, and correctly handles NULL values.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR fixes a crash when binding Python Decimal values to SQL DECIMAL/NUMERIC using setinputsizes() (notably with executemany) by switching to string-based binding and adding regression tests.

Changes:

• Map SQL DECIMAL/NUMERIC to SQL_C_CHAR for C-type binding and convert Python Decimal values to strings during parameter processing.
• Update executemany preprocessing to convert Decimal values when SQL type is DECIMAL/NUMERIC.
• Add new cursor tests covering setinputsizes() + SQL_DECIMAL/SQL_NUMERIC with executemany, execute, and NULL.

Reviewed changes

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

File	Description
mssql_python/cursor.py	Switches DECIMAL/NUMERIC binding to SQL_C_CHAR and adds Decimal-to-string conversions in parameter handling paths.
tests/test_004_cursor.py	Adds regression tests validating Decimal binding via setinputsizes() for execute/executemany and NULL handling.

---

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

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 77%

🎯 Overall Coverage 79%

📈 Total Lines Covered: 6701 out of 8454 📁 Project: mssql-python

---

Diff Coverage

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

• mssql_python/cursor.py (77.8%): Missing lines 2327-2328

Summary

• Total: 9 lines
• Missing: 2 lines
• Coverage: 77%

mssql_python/cursor.py

Lines 2323-2332

  2323                         processed_row[i] = format(val, "f")
  2324                     else:
  2325                         try:
  2326                             processed_row[i] = format(decimal.Decimal(str(val)), "f")
! 2327                         except Exception as e:  # pylint: disable=broad-exception-caught
! 2328                             raise ValueError(
  2329                                 f"Failed to convert parameter at row {row}, column {i} to Decimal: {e}"
  2330                             ) from e
  2331             processed_parameters.append(processed_row)

---

---

📋 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: 67.8%
mssql_python.row.py: 70.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 74.5%
mssql_python.pybind.connection.connection.cpp: 75.8%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.connection.py: 85.2%

---

🔗 Quick Links

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

[bewithgaurav]

core fix lgtm, have added some commenst around tests, suggestions to improve code coverage and a possible unrelated corner case in this path
will await responses

[bewithgaurav]

saw in the coverage report that these lines are uncovered
for the except branch that fires when a non-Decimal value in a DECIMAL/NUMERIC column can't be converted via decimal.Decimal(str(val)), you need to pass an unconvertible value (e.g., a non-numeric string) to a column typed as SQL_DECIMAL - this might cover this branch

[bewithgaurav]

while checking cases, there's a corner case for MONEY type (pre-existing - didn't cause due to this PR) which is:

    Decimal("1000000000000000.00")   (outside MONEY range, no setinputsizes)
         │
    _map_sql_type() → paramCType = SQL_C_NUMERIC  (expects NumericData)
         │
    processing loop → format(val, "f") → string "1000000000000000.00"
         │
    C++ binding → "Is this NumericData?" → NO → RuntimeError

this might be out of scope of this PR but we need to address
we can start a new issue for this - will let you take a call

[bewithgaurav]

suggesting some more additional tests:

1. High-precision Decimal via setinputsizes

   • What: Test Decimal("12345678901234567890.123456789012345678") with setinputsizes([(SQL_DECIMAL, 38, 18)])
   • Why: Verifies string-based binding preserves full DECIMAL(38,18) precision
   • How: Insert high-precision value, read back, compare

2. Negative zero

   • What: Test Decimal("-0.00") with setinputsizes(SQL_DECIMAL)
   • Why: format(Decimal("-0.00"), "f") produces "-0.00" - verify SQL Server handles this

3. Mixed NULL/non-NULL in executemany

   • What: executemany with some rows having None for the DECIMAL column, others having Decimal values
   • Why: Exercises the NULL handling path alongside the Decimal→string conversion in the same batch

[sumitmsft]

Changing SQL_DECIMAL/SQL_NUMERIC from SQL_C_NUMERIC to SQL_C_CHAR is a behavioral change that affects all decimal/numeric bindings globally, not just setinputsizes paths. Any downstream code or user code that relied on the previous SQL_C_NUMERIC binding behavior (e.g., receiving NumericData structs) will now receive strings instead. This is the most impactful change in the PR.

Ensure integration tests cover the non-setinputsizes path (plain execute with Decimal parameters without calling setinputsizes) to confirm no regression. The MONEY corner case flagged by @bewithgaurav is a concrete example of potential fallout.