FIX: bind executemany money-range Decimals as SQL_NUMERIC

[VyrnSynx (vyrnsynx)]

Work Item / Issue Reference

GitHub Issue: #745

---

Summary

executemany auto-detect still used the MONEY/SMALLMONEY-range VARCHAR shortcut after #742 fixed execute(). A money-range Decimal compared against a smaller numeric column could still overflow on that path.

This change binds auto-detected Decimal columns as SQL_NUMERIC with a batch-wide precision/scale (values still go through the existing SQL_C_CHAR string conversion). The setinputsizes DECIMAL/NUMERIC string path (GH-503) and mixed-sign VARCHAR sizing (GH-557) are left alone.

Fixes #745

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🟡 Changes recommended

The new batch precision/scale path needs stricter validation and the current executemany numeric override still conflates numeric precision with string buffer sizing in a way that can produce invalid SQL Server precision (>38) for some Decimal shapes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes the remaining executemany() auto-detection path where MONEY/SMALLMONEY-range Decimal values could still be bound as SQL_VARCHAR, causing SQL Server to overflow when comparing against smaller numeric/decimal columns. The change aligns executemany() with the already-fixed execute() behavior by binding auto-detected Decimal columns as SQL_NUMERIC using a batch-wide precision/scale, while still sending values as SQL_C_CHAR strings.

Changes:

• Update executemany() auto-detect to bind Decimal columns as SQL_NUMERIC (skipping the MONEY-range VARCHAR shortcut) and compute a batch-wide (precision, scale).
• Add unit/integration tests covering the GH-745 overflow regression and mixed-sign batch behavior.
• Document the behavior change in the changelog and update explanatory test/module docs.

File summaries

File	Description
mssql_python/cursor.py	Adds batch-wide Decimal precision/scale derivation and applies it to executemany() auto-detect numeric binding.
tests/test_020_money_smallmoney.py	Adds DB integration coverage for GH-745 (no overflow on executemany comparisons; mixed-sign batch still works).
tests/test_004_cursor.py	Adds unit tests for batch precision/scale derivation and verifies executemany binds money-range Decimals as SQL_NUMERIC.
CHANGELOG.md	Records GH-745 fix and clarifies setinputsizes behavior remains unchanged.

Review details

Suppressed comments (1)

mssql_python/cursor.py:2729

• executemany’s SQL_NUMERIC/SQL_DECIMAL override reuses ParamInfo.columnSize both as the numeric precision (passed as cbColDef to SQLBindParameter) and as the max SQL_C_CHAR buffer length. Setting columnSize to max_decimal_len (which includes sign and decimal point) can push the declared numeric precision above SQL Server’s max 38 (e.g., scale-38 values format to 40 chars), causing bind failures unrelated to the actual numeric precision/scale. Consider decoupling numeric precision from string buffer sizing (e.g., compute buffer sizes from actual encoded string lengths in BindParameterArray when SQLType is NUMERIC/DECIMAL but keep cbColDef=precision<=38).

                    # Ensure columnSize also accommodates the longest string form
                    # (mixed-sign batches, GH-557).
                    if max_decimal_len > paraminfo.columnSize:
                        paraminfo.columnSize = max_decimal_len

• 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.

[Sumit Sarabhai (sumitmsft)]

Reviewed the change that routes executemany auto-detected Decimal columns to SQL_NUMERIC (GH-745). The core approach is sound and the new _decimal_sql_precision_scale / _batch_decimal_precision_scale helpers correctly derive a batch-wide precision/scale (and fix a latent sample-only sizing gap). Verified the precision/scale math matches _get_numeric_data, and black --check passes.

One blocking issue: the retained max_decimal_len override inside the NUMERIC block sets the NUMERIC precision from a formatted string length, which can push precision past 38 and break near-max-precision money-range Decimals that previously worked. Two smaller reliability points below.

Recommendation: Request changes.

[Sumit Sarabhai (sumitmsft)]

For a SQL_NUMERIC param, columnSize is the numeric precision passed to SQLBindParameter (ddbc_bindings.cpp, 6th arg), not a string length. The max_decimal_len > columnSize override just below counts the decimal point and sign, so it sets precision to batch_precision + 1/2. Now that money-range Decimals flow into this NUMERIC block, a full-scale value like Decimal("1E-38") or Decimal("0."+"1"*38) becomes NUMERIC(40,38) and the driver rejects it (max precision 38), a regression from the prior VARCHAR path and inconsistent with the execute() fix (GH-740). batch_precision already sizes precision correctly; the string-length need is handled separately in the SQL_VARCHAR block below. Please drop this override in the NUMERIC branch and add an executemany test for Decimal("1E-38") into NUMERIC(38,38).

[Sumit Sarabhai (sumitmsft)]

batch_precision is applied to columnSize without a <=38 check. The sample-level guard in _map_sql_type doesn't cover the whole batch, so a mixed batch (many integer digits in one row, many fractional in another) can produce batch_precision > 38 even when each value is individually storable, and the driver then fails with an opaque message. Consider raising the existing "precision too high" ValueError when batch_precision > 38. Don't clamp, since that would silently truncate.

[Sumit Sarabhai (sumitmsft)]

Deriving decimal_as_numeric from just the sample type forces the whole column onto the NUMERIC/string-decimal path. A heterogeneous column (Decimal sample plus a stray non-numeric value) that previously bound as VARCHAR will now raise in the Decimal conversion loop below. Consider gating this on "all non-NULL values are Decimal," or documenting heterogeneous columns as unsupported on this path.

[VyrnSynx (vyrnsynx)]

Sumit Sarabhai (@sumitmsft) Thanks for the review — addressed in the latest commit:

1. Dropped the max_decimal_len override inside the NUMERIC branch so columnSize stays as numeric precision (not string length).
2. Raise ValueError when batch precision exceeds 38 (no clamp).
3. Force decimal_as_numeric only when every non-NULL value in the column is a Decimal.

Also added unit coverage for near-max precision (1E-38), batch precision >38, heterogeneous columns, and non-finite Decimals.

[VyrnSynx (vyrnsynx)]

@microsoft-github-policy-service agree