From 13147b226c3ecba48c577fd9630295f51a41dfe6 Mon Sep 17 00:00:00 2001 From: vyrnsynx <153433026+vyrnsynx@users.noreply.github.com> Date: Fri, 4 Sep 2026 04:42:56 +0000 Subject: [PATCH 1/2] FIX: bind executemany money-range Decimals as SQL_NUMERIC executemany auto-detect skipped the money-range VARCHAR shortcut by deriving a batch-wide NUMERIC precision/scale, matching execute() so comparisons against smaller numeric columns no longer overflow. setinputsizes DECIMAL/NUMERIC string binding is unchanged. Fixes #745 --- CHANGELOG.md | 1 + mssql_python/cursor.py | 83 ++++++++++++++++++++++++++---- tests/test_004_cursor.py | 57 ++++++++++++++++++++ tests/test_020_money_smallmoney.py | 65 ++++++++++++++++++++++- 4 files changed, 193 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28e2469db..e19c99926 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), before; users should call `cursor.setinputsizes()` to work around this. ### Fixed +- **GH-745:** `executemany` auto-detect binds money-range `Decimal` values as `SQL_NUMERIC` with a batch-wide precision/scale (still via `SQL_C_CHAR` string values), so a comparison against a smaller numeric column no longer overflows. The `setinputsizes` DECIMAL/NUMERIC string path is unchanged. - **GH-740:** A Python `Decimal` whose value falls in the SQL Server MONEY / SMALLMONEY range is now bound as `SQL_NUMERIC` with its own precision and scale on both `execute()` paths (native detection, and the legacy path reached when diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 02915a952..cb91bafd4 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -661,9 +661,9 @@ def _map_sql_type( # pylint: disable=too-many-arguments,too-many-positional-arg - i: The index of the parameter in the list. - decimal_as_numeric: When True, bind a Decimal as SQL_NUMERIC regardless of value, skipping the MONEY/SMALLMONEY-range VARCHAR shortcut. The execute() - path sets this so a money-range Decimal compared against a numeric column - does not overflow (GH-740). executemany() leaves it False because it - string-binds Decimals for the whole batch (GH-503). + path and executemany() auto-detect path set this so a money-range Decimal + compared against a numeric column does not overflow (GH-740, GH-745). + setinputsizes DECIMAL/NUMERIC still string-binds (GH-503). Returns: - A tuple containing the SQL type, C type, column size, and decimal digits. """ @@ -799,11 +799,11 @@ def _map_sql_type( # pylint: disable=too-many-arguments,too-many-positional-arg f"The maximum precision supported by SQL Server is 38, but got {precision}." ) - # Detect MONEY / SMALLMONEY range. Skipped on the execute() path - # (decimal_as_numeric=True), where a money-range Decimal must bind as - # SQL_NUMERIC so a comparison against a smaller numeric column returns no - # match instead of overflowing (GH-740). executemany keeps the VARCHAR - # shortcut because it string-binds Decimals for the batch (GH-503). + # Detect MONEY / SMALLMONEY range. Skipped when decimal_as_numeric=True + # (execute() and executemany auto-detect), where a money-range Decimal must + # bind as SQL_NUMERIC so a comparison against a smaller numeric column + # returns no match instead of overflowing (GH-740, GH-745). The + # setinputsizes DECIMAL path still string-binds (GH-503). if not decimal_as_numeric and SMALLMONEY_MIN <= param <= SMALLMONEY_MAX: logger.debug("_map_sql_type: DECIMAL -> SMALLMONEY - index=%d", i) # smallmoney @@ -1845,8 +1845,7 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state for i, param in enumerate(parameters): # decimal_as_numeric=True so an uncovered money-range Decimal here # (setinputsizes shorter than the parameter list) binds as SQL_NUMERIC - # like the native path, not VARCHAR (GH-740). executemany keeps the - # VARCHAR shortcut for its batch string binding (GH-503). + # like the native path, not VARCHAR (GH-740). paraminfo = self._create_parameter_types_list( param, param_info, parameters, i, decimal_as_numeric=True ) @@ -2371,6 +2370,52 @@ def _transpose_rowwise_to_columnwise( return columnwise, row_count + @staticmethod + def _decimal_sql_precision_scale(value: decimal.Decimal) -> Tuple[int, int]: + """Return SQL NUMERIC (precision, scale) for a finite Decimal. + + Matches the precision/scale rules used by _map_sql_type / _get_numeric_data. + """ + decimal_as_tuple = value.as_tuple() + digits_tuple = decimal_as_tuple.digits + num_digits = len(digits_tuple) + exponent = decimal_as_tuple.exponent + if isinstance(exponent, str): + raise ValueError("Cannot bind non-finite Decimal (NaN/Infinity) as SQL NUMERIC") + if exponent >= 0: + precision = num_digits + exponent + scale = 0 + elif (-1 * exponent) <= num_digits: + precision = num_digits + scale = exponent * -1 + else: + precision = exponent * -1 + scale = exponent * -1 + return precision, scale + + def _batch_decimal_precision_scale(self, column) -> Tuple[int, int]: + """Derive one NUMERIC(precision, scale) that fits every Decimal in a column. + + Used by executemany so money-range Decimals can bind as SQL_NUMERIC with a + single batch-wide type (GH-745) without shrinking any row's digits. + """ + max_scale = 0 + max_int_digits = 0 + found = False + for value in column: + if not isinstance(value, decimal.Decimal): + continue + try: + precision, scale = self._decimal_sql_precision_scale(value) + except ValueError: + continue + found = True + max_scale = max(max_scale, scale) + max_int_digits = max(max_int_digits, precision - scale) + if not found: + return 0, 0 + return max(max_int_digits + max_scale, 1), max_scale + def _compute_column_type(self, column): """ Determine representative value and integer min/max for a column. @@ -2638,6 +2683,13 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s ) sample_value, min_val, max_val, max_decimal_len = self._compute_column_type(column) + # GH-745: auto-detected Decimal columns bind as SQL_NUMERIC (skipping + # the money-range VARCHAR shortcut) so a money-range value compared + # against a smaller numeric column does not overflow. executemany still + # string-binds via SQL_C_CHAR below; setinputsizes DECIMAL stays on the + # GH-503 string path above. + decimal_as_numeric = isinstance(sample_value, decimal.Decimal) + dummy_row = list(sample_row) paraminfo = self._create_parameter_types_list( sample_value, @@ -2646,6 +2698,7 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s col_index, min_val=min_val, max_val=max_val, + decimal_as_numeric=decimal_as_numeric, ) # GH-610: all-NULL columns now pass SQL_UNKNOWN_TYPE to C++, @@ -2663,7 +2716,15 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s ddbc_sql_const.SQL_NUMERIC.value, ): paraminfo.paramCType = ddbc_sql_const.SQL_C_CHAR.value - # Ensure columnSize accommodates the longest string representation + # One NUMERIC(precision, scale) must fit every Decimal in the + # batch (GH-745). Sample-only precision/scale is not enough. + batch_precision, batch_scale = self._batch_decimal_precision_scale(column) + if batch_precision > paraminfo.columnSize: + paraminfo.columnSize = batch_precision + if batch_scale > paraminfo.decimalDigits: + paraminfo.decimalDigits = batch_scale + # Ensure columnSize also accommodates the longest string form + # (mixed-sign batches, GH-557). if max_decimal_len > paraminfo.columnSize: paraminfo.columnSize = max_decimal_len diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index a3ddaaa1f..2fa714b74 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -17182,6 +17182,63 @@ def test_map_sql_type_decimal_in_money_returns_varchar(): assert c_type == _C.SQL_C_CHAR.value +def test_gh745_batch_decimal_precision_scale_covers_all_rows(): + """_batch_decimal_precision_scale fits every Decimal in the column.""" + cur = _make_bare_cursor() + column = [ + decimal.Decimal("1.0"), + decimal.Decimal("12345.6789"), + decimal.Decimal("-0.1"), + ] + precision, scale = cur._batch_decimal_precision_scale(column) + assert scale >= 4 + assert precision >= 9 + + +def test_gh745_executemany_money_range_binds_as_numeric(monkeypatch): + """executemany auto-detect binds money-range Decimals as SQL_NUMERIC (GH-745).""" + from unittest.mock import MagicMock + from mssql_python import ddbc_bindings + from mssql_python.cursor import Cursor + + cur = Cursor.__new__(Cursor) + cur._inputsizes = None + cur._timeout = 0 + cur.closed = False + cur.hstmt = MagicMock() + cur.messages = [] + cur.is_stmt_prepared = [False] + cur._connection = MagicMock() + cur._connection._encoding = "utf-8" + cur._connection._conn = MagicMock() + captured = {} + + def fake_sql_execute_many(hstmt, op, col_params, param_types, row_count, enc): + captured["parameters_type"] = param_types + captured["columnwise_params"] = col_params + return 0 + + monkeypatch.setattr(cur, "_check_closed", lambda: None) + monkeypatch.setattr(cur, "_reset_cursor", lambda: None) + monkeypatch.setattr(ddbc_bindings, "SQLExecuteMany", fake_sql_execute_many) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLGetAllDiagRecords", lambda h: []) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLRowCount", lambda h: 2) + data = [ + (decimal.Decimal("12.34"),), + (decimal.Decimal("12345.6789"),), + (decimal.Decimal("-0.1"),), + ] + cur.executemany("UPDATE t SET x = 1 WHERE v = ?", data) + pt = captured["parameters_type"] + assert len(pt) == 1 + assert pt[0].paramSQLType == _C.SQL_NUMERIC.value + assert pt[0].paramCType == _C.SQL_C_CHAR.value + assert pt[0].columnSize >= len("-0.1") + assert pt[0].decimalDigits >= 4 + for val in captured["columnwise_params"][0]: + assert isinstance(val, str) + + def test_executemany_numeric_override_needed(): """The executemany auto-detection path must override SQL_C_NUMERIC to SQL_C_CHAR (GH-609).""" from mssql_python import ddbc_bindings diff --git a/tests/test_020_money_smallmoney.py b/tests/test_020_money_smallmoney.py index f60d37e00..848add1c9 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 auto-detect likewise binds Decimals as SQL_NUMERIC with +a batch-wide precision/scale and SQL_C_CHAR string values (GH-745); setinputsizes +DECIMAL/NUMERIC still string-binds for fixed precision (GH-503). """ import pytest @@ -812,3 +813,63 @@ def test_gh740_signed_zero_normalizes(cursor, db_connection): finally: drop_table_if_exists(cursor, table_name) db_connection.commit() + + +# ============================================================================= +# GH-745: executemany money-range Decimal must bind as SQL_NUMERIC, not VARCHAR +# ============================================================================= + + +def test_gh745_executemany_in_range_decimal_numeric_comparison_no_overflow(cursor, db_connection): + """executemany must not overflow money-range Decimals against a smaller numeric. + + Before the fix, executemany still used the MONEY-range VARCHAR shortcut, so + SQL Server did a varchar->numeric conversion that overflowed instead of simply + not matching (the execute() path was fixed in GH-740 / #742). + """ + table_name = "#pytest_gh745_cmp" + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (v numeric(5,2))") # max 999.99 + cursor.execute(f"INSERT INTO {table_name} VALUES (?)", [Decimal("12.34")]) + db_connection.commit() + + # Comparison via executemany is an unnatural shape, but it is the path that + # still carried the VARCHAR shortcut. UPDATE ... WHERE keeps the binding. + cursor.executemany( + f"UPDATE {table_name} SET v = v WHERE v = ?", + [(Decimal("12345.6789"),), (Decimal("300000.00"),)], + ) + cursor.execute(f"SELECT COUNT(*) FROM {table_name}") + assert cursor.fetchone()[0] == 1 + + cursor.executemany( + f"UPDATE {table_name} SET v = v WHERE v = ?", + [(Decimal("12.34"),)], + ) + cursor.execute(f"SELECT COUNT(*) FROM {table_name} WHERE v = ?", [Decimal("12.34")]) + assert cursor.fetchone()[0] == 1 + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit() + + +def test_gh745_executemany_mixed_sign_money_range_batch(cursor, db_connection): + """Mixed-sign money-range Decimals still insert through executemany (GH-557).""" + table_name = "#pytest_gh745_sign" + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (v DECIMAL(28, 14))") + data = [ + (Decimal("1.0"),), + (Decimal("-0.1"),), + (Decimal("100.5"),), + (Decimal("-999.99"),), + ] + cursor.executemany(f"INSERT INTO {table_name} VALUES (?)", data) + db_connection.commit() + cursor.execute(f"SELECT COUNT(*) FROM {table_name}") + assert cursor.fetchone()[0] == 4 + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit() From 04a6ab75ed4fa3f6c4a12072e721cb2b09c3dff6 Mon Sep 17 00:00:00 2001 From: vyrnsynx <153433026+vyrnsynx@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:08:29 +0000 Subject: [PATCH 2/2] FIX: drop NUMERIC max_decimal_len override and harden batch Decimal binding Address review feedback on the executemany SQL_NUMERIC path: keep columnSize as numeric precision (not string length), raise when batch precision exceeds 38, force NUMERIC only when every non-NULL value is Decimal, and cover the cases with unit tests. --- mssql_python/cursor.py | 28 +++++++--- tests/test_004_cursor.py | 117 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 10 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index cb91bafd4..0e50f8751 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2398,6 +2398,9 @@ def _batch_decimal_precision_scale(self, column) -> Tuple[int, int]: Used by executemany so money-range Decimals can bind as SQL_NUMERIC with a single batch-wide type (GH-745) without shrinking any row's digits. + + Non-finite Decimals (NaN/Infinity) raise ValueError rather than being + skipped. Callers must enforce SQL Server's precision limit (<= 38). """ max_scale = 0 max_int_digits = 0 @@ -2405,10 +2408,8 @@ def _batch_decimal_precision_scale(self, column) -> Tuple[int, int]: for value in column: if not isinstance(value, decimal.Decimal): continue - try: - precision, scale = self._decimal_sql_precision_scale(value) - except ValueError: - continue + # Propagate non-finite errors; do not silently skip them. + precision, scale = self._decimal_sql_precision_scale(value) found = True max_scale = max(max_scale, scale) max_int_digits = max(max_int_digits, precision - scale) @@ -2688,7 +2689,12 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s # against a smaller numeric column does not overflow. executemany still # string-binds via SQL_C_CHAR below; setinputsizes DECIMAL stays on the # GH-503 string path above. - decimal_as_numeric = isinstance(sample_value, decimal.Decimal) + # Only force NUMERIC when every non-NULL value in the column is Decimal; + # a heterogeneous column keeps the prior sample-driven path. + non_null_values = [v for v in column if v is not None] + decimal_as_numeric = bool(non_null_values) and all( + isinstance(v, decimal.Decimal) for v in non_null_values + ) dummy_row = list(sample_row) paraminfo = self._create_parameter_types_list( @@ -2718,15 +2724,19 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s paraminfo.paramCType = ddbc_sql_const.SQL_C_CHAR.value # One NUMERIC(precision, scale) must fit every Decimal in the # batch (GH-745). Sample-only precision/scale is not enough. + # columnSize is NUMERIC precision for SQLBindParameter, not a + # string buffer length — do not widen it with max_decimal_len. batch_precision, batch_scale = self._batch_decimal_precision_scale(column) + if batch_precision > 38: + raise ValueError( + "Precision of the numeric value is too high. " + "The maximum precision supported by SQL Server is 38, " + f"but got {batch_precision}." + ) if batch_precision > paraminfo.columnSize: paraminfo.columnSize = batch_precision if batch_scale > paraminfo.decimalDigits: paraminfo.decimalDigits = batch_scale - # Ensure columnSize also accommodates the longest string form - # (mixed-sign batches, GH-557). - if max_decimal_len > paraminfo.columnSize: - paraminfo.columnSize = max_decimal_len # Correct column size for Decimal columns sent as SQL_VARCHAR (GH-557). # The sample value's formatted string may be shorter than another diff --git a/tests/test_004_cursor.py b/tests/test_004_cursor.py index 2fa714b74..3b616a79d 100644 --- a/tests/test_004_cursor.py +++ b/tests/test_004_cursor.py @@ -17233,12 +17233,127 @@ def fake_sql_execute_many(hstmt, op, col_params, param_types, row_count, enc): assert len(pt) == 1 assert pt[0].paramSQLType == _C.SQL_NUMERIC.value assert pt[0].paramCType == _C.SQL_C_CHAR.value - assert pt[0].columnSize >= len("-0.1") + # columnSize is NUMERIC precision (not formatted-string length). + assert pt[0].columnSize == 9 + assert pt[0].columnSize <= 38 assert pt[0].decimalDigits >= 4 for val in captured["columnwise_params"][0]: assert isinstance(val, str) +def test_gh745_batch_decimal_rejects_non_finite(): + """_batch_decimal_precision_scale must not silently skip NaN/Infinity.""" + cur = _make_bare_cursor() + column = [decimal.Decimal("1.0"), decimal.Decimal("NaN")] + with pytest.raises(ValueError, match="non-finite"): + cur._batch_decimal_precision_scale(column) + + +def test_gh745_executemany_near_max_precision_stays_within_38(monkeypatch): + """NUMERIC columnSize must stay <= 38 for near-max Decimals (no max_decimal_len).""" + from unittest.mock import MagicMock + from mssql_python import ddbc_bindings + from mssql_python.cursor import Cursor + + cur = Cursor.__new__(Cursor) + cur._inputsizes = None + cur._timeout = 0 + cur.closed = False + cur.hstmt = MagicMock() + cur.messages = [] + cur.is_stmt_prepared = [False] + cur._connection = MagicMock() + cur._connection._encoding = "utf-8" + cur._connection._conn = MagicMock() + captured = {} + + def fake_sql_execute_many(hstmt, op, col_params, param_types, row_count, enc): + captured["parameters_type"] = param_types + return 0 + + monkeypatch.setattr(cur, "_check_closed", lambda: None) + monkeypatch.setattr(cur, "_reset_cursor", lambda: None) + monkeypatch.setattr(ddbc_bindings, "SQLExecuteMany", fake_sql_execute_many) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLGetAllDiagRecords", lambda h: []) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLRowCount", lambda h: 1) + # Decimal('1E-38') needs precision=38, scale=38. Formatted string length is > 38, + # so the old max_decimal_len override would wrongly push precision past 38. + data = [(decimal.Decimal("1E-38"),)] + cur.executemany("UPDATE t SET x = 1 WHERE v = ?", data) + pt = captured["parameters_type"][0] + assert pt.paramSQLType == _C.SQL_NUMERIC.value + assert pt.columnSize == 38 + assert pt.decimalDigits == 38 + + +def test_gh745_executemany_batch_precision_over_38_raises(monkeypatch): + """Mixed batch whose combined precision exceeds 38 must raise ValueError.""" + from unittest.mock import MagicMock + from mssql_python import ddbc_bindings + from mssql_python.cursor import Cursor + + cur = Cursor.__new__(Cursor) + cur._inputsizes = None + cur._timeout = 0 + cur.closed = False + cur.hstmt = MagicMock() + cur.messages = [] + cur.is_stmt_prepared = [False] + cur._connection = MagicMock() + cur._connection._encoding = "utf-8" + cur._connection._conn = MagicMock() + + monkeypatch.setattr(cur, "_check_closed", lambda: None) + monkeypatch.setattr(cur, "_reset_cursor", lambda: None) + monkeypatch.setattr(ddbc_bindings, "SQLExecuteMany", lambda *a, **k: 0) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLGetAllDiagRecords", lambda h: []) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLRowCount", lambda h: 0) + # 20 integer digits + 20 fractional digits across rows => batch precision 40. + data = [ + (decimal.Decimal("1" * 20),), + (decimal.Decimal("0." + ("1" * 20)),), + ] + with pytest.raises(ValueError, match="maximum precision supported by SQL Server is 38"): + cur.executemany("UPDATE t SET x = 1 WHERE v = ?", data) + + +def test_gh745_executemany_heterogeneous_column_skips_numeric_force(monkeypatch): + """A Decimal sample plus a non-Decimal value must not force the NUMERIC path.""" + from unittest.mock import MagicMock + from mssql_python import ddbc_bindings + from mssql_python.cursor import Cursor + + cur = Cursor.__new__(Cursor) + cur._inputsizes = None + cur._timeout = 0 + cur.closed = False + cur.hstmt = MagicMock() + cur.messages = [] + cur.is_stmt_prepared = [False] + cur._connection = MagicMock() + cur._connection._encoding = "utf-8" + cur._connection._conn = MagicMock() + captured = {} + + def fake_sql_execute_many(hstmt, op, col_params, param_types, row_count, enc): + captured["parameters_type"] = param_types + return 0 + + monkeypatch.setattr(cur, "_check_closed", lambda: None) + monkeypatch.setattr(cur, "_reset_cursor", lambda: None) + monkeypatch.setattr(ddbc_bindings, "SQLExecuteMany", fake_sql_execute_many) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLGetAllDiagRecords", lambda h: []) + monkeypatch.setattr(ddbc_bindings, "DDBCSQLRowCount", lambda h: 2) + data = [ + (decimal.Decimal("12.34"),), + ("not-a-decimal",), + ] + cur.executemany("UPDATE t SET x = 1 WHERE v = ?", data) + pt = captured["parameters_type"][0] + # Sample is Decimal but column is heterogeneous — stay off the forced NUMERIC path. + assert pt.paramSQLType != _C.SQL_NUMERIC.value + + def test_executemany_numeric_override_needed(): """The executemany auto-detection path must override SQL_C_NUMERIC to SQL_C_CHAR (GH-609).""" from mssql_python import ddbc_bindings