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..59339bcc1 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2371,6 +2371,44 @@ 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 + # 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( + (-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 +2599,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..bc3a4b585 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,92 @@ def test_executemany_money_smallmoney(cursor, db_connection): db_connection.commit() +class _ParamInfo: + """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 + 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 + + # 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 + 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 # =============================================================================