From 93ef1bd831aba144bfece36c8904512408f043f8 Mon Sep 17 00:00:00 2001 From: IMGillusion Date: Thu, 10 Sep 2026 12:10:45 +0800 Subject: [PATCH 1/2] GH-745: bind money-range Decimal batches as SQL_NUMERIC in executemany --- CHANGELOG.md | 13 +++++- mssql_python/cursor.py | 39 +++++++++++++++++ tests/test_020_money_smallmoney.py | 68 +++++++++++++++++++++++++++++- 3 files changed, 117 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4ebfb816..8a8b4016b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,7 +85,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), `money`/`varchar` in data-type precedence, `WHERE money_or_varchar_col = ?` can add a `CONVERT_IMPLICIT` on the column side that turns an index seek into a scan. `executemany` intentionally keeps its batch `VARCHAR` string binding (GH-503); - the remaining money-range case there is tracked in #745. + the remaining money-range case there is tracked in #745 (fixed below). +- **GH-745:** A `Decimal` batch in the MONEY / SMALLMONEY range passed to + `executemany()` is now re-declared as `SQL_NUMERIC` (with the batch-wide maximum + scale) instead of keeping the sample row's `VARCHAR` money shortcut, mirroring + the GH-740 fix on the `execute()` paths. Previously such a batch compared + against a smaller `numeric`/`decimal` column (`WHERE v = ?`) made SQL Server + convert each row's `varchar`→`numeric` and raise an arithmetic overflow. The + batch still string-binds (GH-503), and `decimalDigits` uses the max scale across + the whole batch so mixed-scale rows all fit the declared type (a sample-only + switch would regress the GH-557 mixed-sign sizing). Only pure-Decimal columns + fully inside the money range are affected; wider values, mixed-type columns, and + `setinputsizes()`-typed columns are untouched. - **GH-725:** The `timeout` parameter of `connect()` / `Connection(...)` now correctly sets the **login (connection-attempt) timeout** (`SQL_ATTR_LOGIN_TIMEOUT`), matching pyodbc and its own docstring. Previously diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 605120bfc..dc1fa537c 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2371,6 +2371,38 @@ def _compute_column_type(self, column): return sample_value, None, None, max_decimal_formatted_len + def _apply_money_batch_declared_type(self, paraminfo, column): + """GH-745: re-declare a money-range Decimal column as SQL_NUMERIC. + + The auto-detection sample path takes the VARCHAR money shortcut + (see _map_sql_type), but a money-range Decimal compared against a + smaller numeric column overflows on the server side when bound as + VARCHAR - the same bug GH-740 fixed on the execute() paths. Declare + SQL_NUMERIC instead; the existing override below keeps the + SQL_C_CHAR string binding (GH-503), and decimalDigits uses the + batch-wide max scale so every row fits the declared type (a naive + sample-only switch regresses mixed-scale batches, GH-557 shape). + + Returns True if the declaration was changed. + """ + if paraminfo.paramSQLType != ddbc_sql_const.SQL_VARCHAR.value: + return False + non_nulls = [v for v in column if v is not None] + if not non_nulls or not all(isinstance(v, decimal.Decimal) for v in non_nulls): + return False + if not all(SMALLMONEY_MIN <= v <= MONEY_MAX for v in non_nulls): + return False + paraminfo.paramSQLType = ddbc_sql_const.SQL_NUMERIC.value + paraminfo.decimalDigits = max( + (-v.as_tuple().exponent) if v.as_tuple().exponent < 0 else 0 for v in non_nulls + ) + logger.debug( + "executemany: money-range batch re-declared as SQL_NUMERIC, " + "decimalDigits=%d (GH-745)", + paraminfo.decimalDigits, + ) + return True + def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-statements self, operation: str, @@ -2561,6 +2593,13 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s max_val=max_val, ) + # GH-745: a money-range Decimal batch declared as SQL_VARCHAR + # overflows on the server when compared against a smaller + # numeric column (the executemany remnant of GH-740). Re-declare + # as SQL_NUMERIC with batch-wide scale; string binding (GH-503) + # is preserved by the override below. + self._apply_money_batch_declared_type(paraminfo, column) + # GH-610: all-NULL columns now pass SQL_UNKNOWN_TYPE to C++, # where BindParameterArray resolves the correct type via the # SQLDescribeParam cache. The previous SQL_VARCHAR hardcoded diff --git a/tests/test_020_money_smallmoney.py b/tests/test_020_money_smallmoney.py index f60d37e00..62195cf2d 100644 --- a/tests/test_020_money_smallmoney.py +++ b/tests/test_020_money_smallmoney.py @@ -8,8 +8,9 @@ SQL_NUMERIC using its own precision and scale, regardless of value. Binding no longer depends on whether the value falls in the MONEY/SMALLMONEY range, so an in-range value compared against a smaller numeric column returns no match instead of a varchar->numeric -overflow (GH-740). executemany still string-binds Decimals (SQL_VARCHAR) to preserve -scale-38 precision (GH-503), so that path is unchanged here. +overflow (GH-740). executemany string-binds Decimals to preserve scale-38 precision +(GH-503); a money-range batch is additionally re-declared as SQL_NUMERIC with batch-wide +scale so the server does not overflow when coercing the strings (GH-745). """ import pytest @@ -528,6 +529,69 @@ def test_executemany_money_smallmoney(cursor, db_connection): db_connection.commit() +class _ParamInfo: + """Duck-typed stand-in for ddbc_bindings.ParamInfo (no C extension needed).""" + + def __init__(self, sql_type): + self.paramSQLType = sql_type + self.decimalDigits = 0 + self.columnSize = 0 + + +def _bare_cursor(): + """A Cursor instance that skips __init__ (no DB connection required).""" + from mssql_python.cursor import Cursor + + return Cursor.__new__(Cursor) + + +def test_money_batch_redeclared_as_numeric(): + """GH-745: a money-range Decimal batch must declare SQL_NUMERIC (not the + VARCHAR shortcut), with decimalDigits = batch-wide max scale, so the server + does not overflow converting the strings. Mirrors the GH-740 fix on + execute(); the string binding (GH-503) is applied separately downstream.""" + from mssql_python.constants import ConstantsDDBC as ddbc + + cur = _bare_cursor() + varchar = ddbc.SQL_VARCHAR.value + numeric = ddbc.SQL_NUMERIC.value + + # In-range batch, mixed scales -> redeclared NUMERIC with max scale 4 + p = _ParamInfo(varchar) + assert cur._apply_money_batch_declared_type( + p, [Decimal("12345.6789"), Decimal("-0.0001"), None] + ) is True + assert p.paramSQLType == numeric + assert p.decimalDigits == 4 + + # Integer-valued money -> scale 0 + p = _ParamInfo(varchar) + assert cur._apply_money_batch_declared_type(p, [Decimal("100"), Decimal("-2")]) is True + assert p.paramSQLType == numeric + assert p.decimalDigits == 0 + + # Above MONEY_MAX -> unchanged (existing NUMERIC path handles it) + p = _ParamInfo(varchar) + assert cur._apply_money_batch_declared_type(p, [Decimal("9999999999999999")]) is False + assert p.paramSQLType == varchar + + # Mixed Decimal + str -> unchanged (not a pure Decimal column) + p = _ParamInfo(varchar) + assert cur._apply_money_batch_declared_type(p, [Decimal("1.5"), "2.5"]) is False + assert p.paramSQLType == varchar + + # All-NULL column -> unchanged (nothing to type) + p = _ParamInfo(varchar) + assert cur._apply_money_batch_declared_type(p, [None, None]) is False + assert p.paramSQLType == varchar + + # Already declared non-VARCHAR -> untouched + p = _ParamInfo(numeric) + assert cur._apply_money_batch_declared_type(p, [Decimal("1.5")]) is False + assert p.paramSQLType == numeric + assert p.decimalDigits == 0 + + # ============================================================================= # Invalid Input Handling # ============================================================================= From adda2083634f24bc3b4bbcd1eb4c36ac51580b9b Mon Sep 17 00:00:00 2001 From: IMGillusion Date: Thu, 10 Sep 2026 12:51:10 +0800 Subject: [PATCH 2/2] GH-745: fix money-range floor + Copilot review findings - Lower bound of the redeclare range is MONEY_MIN, not SMALLMONEY_MIN: a value like Decimal('-300000') is in the MONEY range but below SMALLMONEY_MIN and was left as SQL_VARCHAR, reintroducing the overflow on negative money-range values. - Test: cover the negative-money-below-SMALLMONEY_MIN edge and the below-MONEY_MIN boundary; clarify the _ParamInfo docstring. (Copilot columnSize concern is not reachable for the money range: money has <=4 decimal places so the string length is always <38 and the declared precision is never < the scale.) --- mssql_python/cursor.py | 8 +++++++- tests/test_020_money_smallmoney.py | 25 ++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index dc1fa537c..59339bcc1 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2390,7 +2390,13 @@ def _apply_money_batch_declared_type(self, paraminfo, column): non_nulls = [v for v in column if v is not None] if not non_nulls or not all(isinstance(v, decimal.Decimal) for v in non_nulls): return False - if not all(SMALLMONEY_MIN <= v <= MONEY_MAX for v in non_nulls): + # The money shortcut (see _map_sql_type) string-binds any Decimal in + # the full MONEY range; SMALLMONEY is a subset of that range. The + # lower bound must be MONEY_MIN, not SMALLMONEY_MIN: a value like + # Decimal("-300000") is in the MONEY range but below SMALLMONEY_MIN, + # and using the smaller floor left such batches as SQL_VARCHAR, + # reintroducing the GH-745 overflow on negative money-range values. + if not all(MONEY_MIN <= v <= MONEY_MAX for v in non_nulls): return False paraminfo.paramSQLType = ddbc_sql_const.SQL_NUMERIC.value paraminfo.decimalDigits = max( diff --git a/tests/test_020_money_smallmoney.py b/tests/test_020_money_smallmoney.py index 62195cf2d..bc3a4b585 100644 --- a/tests/test_020_money_smallmoney.py +++ b/tests/test_020_money_smallmoney.py @@ -530,7 +530,15 @@ def test_executemany_money_smallmoney(cursor, db_connection): class _ParamInfo: - """Duck-typed stand-in for ddbc_bindings.ParamInfo (no C extension needed).""" + """Duck-typed stand-in for ddbc_bindings.ParamInfo. + + It avoids *constructing* a real ParamInfo (the pure-Python fields + `_apply_money_batch_declared_type` reads are enough), but the + `Cursor` import in `_bare_cursor` still pulls in the compiled + ddbc_bindings extension, so the test runs only where it is built + (CI); arm64 local runs stub the module (see the GH-745 red/green + script) rather than this file. + """ def __init__(self, sql_type): self.paramSQLType = sql_type @@ -570,6 +578,21 @@ def test_money_batch_redeclared_as_numeric(): assert p.paramSQLType == numeric assert p.decimalDigits == 0 + # Negative money-range value below SMALLMONEY_MIN but inside the MONEY + # range: must still redeclare (the MONEY_MIN floor, not the SMALLMONEY + # floor, is the correct lower bound). GH-745 negative-value edge. + p = _ParamInfo(varchar) + assert cur._apply_money_batch_declared_type(p, [Decimal("-300000")]) is True + assert p.paramSQLType == numeric + assert p.decimalDigits == 0 + + # Below MONEY_MIN (negative overflow) -> unchanged (not a money value) + p = _ParamInfo(varchar) + assert cur._apply_money_batch_declared_type( + p, [Decimal("-922337203685477.5809")] + ) is False + assert p.paramSQLType == varchar + # Above MONEY_MAX -> unchanged (existing NUMERIC path handles it) p = _ParamInfo(varchar) assert cur._apply_money_batch_declared_type(p, [Decimal("9999999999999999")]) is False