From f3335348bbea33a6a17d116b93bebcfe92e90eb4 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:14:59 +0530 Subject: [PATCH 1/6] FIX: bind Decimal as SQL_NUMERIC regardless of value The standard execute path chose a Decimal's bind type from its value: anything in the MONEY/SMALLMONEY range was sent as a formatted VARCHAR. Comparing such a value against a smaller numeric column made SQL Server convert varchar to numeric and overflow, so 'WHERE v = ?' raised an arithmetic overflow instead of just not matching. Bind every finite Decimal as SQL_NUMERIC with its own precision and scale, matching pyodbc. Removing the shortcut surfaced a second bug: the numeric parameter's APD record number in SQLSetDescField was hardcoded to 1, so a numeric parameter in any position other than the first wrote its precision/scale onto the wrong record and the driver raised 'Numeric value out of range'. Use the parameter's own 1-based position. Scoped to the single execute() path; executemany still string-binds decimals (GH-503) and is a separate follow-up. (GH-740) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 16 +++- mssql_python/pybind/param_detect.hpp | 49 ++-------- mssql_python/pybind/py_type_cache.hpp | 18 +--- tests/test_020_money_smallmoney.py | 123 +++++++++++++++++++++++++- 4 files changed, 141 insertions(+), 65 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index cd3a45fa6..8c3e40cba 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -851,6 +851,13 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par // Special handling for Numeric type - // https://learn.microsoft.com/en-us/sql/odbc/reference/appendixes/retrieve-numeric-data-sql-numeric-struct-kb222831?view=sql-server-ver16#sql_c_numeric-overview if (paramInfo.paramCType == SQL_C_NUMERIC) { + // The APD record number is the 1-based parameter position, matching the + // SQLBindParameter call above. It was previously hardcoded to 1, so a + // SQL_C_NUMERIC parameter in any position other than the first had its + // precision/scale/data pointer written onto record 1 instead of its own. + // The driver then read the numeric struct with the wrong descriptor and + // raised "Numeric value out of range" (GH-740). + const SQLSMALLINT descRecNum = static_cast(paramIndex + 1); SQLHDESC hDesc = nullptr; rc = SQLGetStmtAttr_ptr(hStmt, SQL_ATTR_APP_PARAM_DESC, &hDesc, 0, NULL); if (!SQL_SUCCEEDED(rc)) { @@ -859,7 +866,8 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par paramIndex, rc); return rc; } - rc = SQLSetDescField_ptr(hDesc, 1, SQL_DESC_TYPE, (SQLPOINTER)SQL_C_NUMERIC, 0); + rc = SQLSetDescField_ptr(hDesc, descRecNum, SQL_DESC_TYPE, + (SQLPOINTER)SQL_C_NUMERIC, 0); if (!SQL_SUCCEEDED(rc)) { LOG("BindParameters: SQLSetDescField(SQL_DESC_TYPE) failed for " "param[%d] - SQLRETURN=%d", @@ -868,7 +876,7 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par } SQL_NUMERIC_STRUCT* numericPtr = reinterpret_cast(dataPtr); rc = SQLSetDescField_ptr( - hDesc, 1, SQL_DESC_PRECISION, + hDesc, descRecNum, SQL_DESC_PRECISION, reinterpret_cast(static_cast(numericPtr->precision)), 0); if (!SQL_SUCCEEDED(rc)) { LOG("BindParameters: SQLSetDescField(SQL_DESC_PRECISION) " @@ -878,7 +886,7 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par } rc = SQLSetDescField_ptr( - hDesc, 1, SQL_DESC_SCALE, + hDesc, descRecNum, SQL_DESC_SCALE, reinterpret_cast(static_cast(numericPtr->scale)), 0); if (!SQL_SUCCEEDED(rc)) { LOG("BindParameters: SQLSetDescField(SQL_DESC_SCALE) failed " @@ -887,7 +895,7 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par return rc; } - rc = SQLSetDescField_ptr(hDesc, 1, SQL_DESC_DATA_PTR, + rc = SQLSetDescField_ptr(hDesc, descRecNum, SQL_DESC_DATA_PTR, reinterpret_cast(numericPtr), 0); if (!SQL_SUCCEEDED(rc)) { LOG("BindParameters: SQLSetDescField(SQL_DESC_DATA_PTR) failed " diff --git a/mssql_python/pybind/param_detect.hpp b/mssql_python/pybind/param_detect.hpp index 6c5d4338d..62d06de3e 100644 --- a/mssql_python/pybind/param_detect.hpp +++ b/mssql_python/pybind/param_detect.hpp @@ -194,8 +194,9 @@ inline bool StartsWithAscii(unsigned int kind, const void* data, Py_ssize_t leng // storage engine's range exactly (TINYINT: 0-255, SMALLINT: -32768..32767, etc.) // 4. String handling inspects UCS kind directly for O(1) ASCII detection rather than // scanning content — critical for bulk insert scenarios with thousands of params. -// 5. MONEY/SMALLMONEY uses exact Decimal comparison (PyObject_RichCompareBool) to avoid -// double-precision boundary errors (e.g., 214748.3647 would round incorrectly as double). +// 5. Every finite Decimal binds as SQL_NUMERIC with its own precision/scale; the value's +// magnitude does not change the bind type, so a comparison against a smaller numeric +// column returns no match instead of a server-side varchar->numeric overflow (GH-740). // --------------------------------------------------------------------------- // // ORDERING MATTERS: @@ -511,45 +512,11 @@ inline std::vector DetectParamTypes(PyObject* params) { std::to_string(precision) + "."); } - // Check SMALLMONEY first, then widen to MONEY, so common small values keep the narrowest - // exact range while still accepting larger fixed-point values supported by SQL Server. - // MONEY/SMALLMONEY: SQL Server stores these as fixed-point integers internally. - // We bind as formatted VARCHAR (e.g., "214748.3647") because SQL_C_NUMERIC can't - // represent the exact money range without precision loss on certain ODBC drivers. - // Use exact Decimal comparison (not double) to avoid boundary misclassification. - bool in_money_range = false; - int cmp_ge = PyObject_RichCompareBool(obj, PyTypeCache::smallmoney_min, Py_GE); - int cmp_le = PyObject_RichCompareBool(obj, PyTypeCache::smallmoney_max, Py_LE); - if (cmp_ge == -1 || cmp_le == -1) throw py::error_already_set(); - if (cmp_ge == 1 && cmp_le == 1) { - in_money_range = true; - } else { - cmp_ge = PyObject_RichCompareBool(obj, PyTypeCache::money_min, Py_GE); - cmp_le = PyObject_RichCompareBool(obj, PyTypeCache::money_max, Py_LE); - if (cmp_ge == -1 || cmp_le == -1) throw py::error_already_set(); - if (cmp_ge == 1 && cmp_le == 1) { - in_money_range = true; - } - } - - if (in_money_range) { - py::object formatted = steal(PyObject_CallMethod(obj, "__format__", "s", "f")); - if (!formatted) throw py::error_already_set(); - info.paramSQLType = SQL_VARCHAR; - info.paramCType = PARAM_C_TYPE_TEXT; - info.columnSize = PyUnicode_GET_LENGTH(formatted.ptr()); - info.decimalDigits = 0; - PyObject* raw = formatted.release().ptr(); - if (PyList_SetItem(params, i, raw) != 0) { - // PyList_SetItem steals (decrefs) the item even on failure, - // so raw is already freed — do NOT Py_DECREF here. - throw py::error_already_set(); - } - continue; - } - - // Build SQL_NUMERIC_STRUCT from the Decimal object. Store as a pybind11-castable - // object in the param list so BindParameters can extract it as NumericData. + // Bind every finite Decimal as SQL_NUMERIC using its own precision and scale, + // regardless of value. The previous MONEY/SMALLMONEY VARCHAR shortcut chose the + // bind type from the value alone, ignoring the target column, so an in-range value + // compared against a smaller numeric column triggered a server-side varchar->numeric + // overflow instead of simply not matching (GH-740). info.paramSQLType = SQL_NUMERIC; info.paramCType = SQL_C_NUMERIC; NumericData nd = build_numeric_data(as_tuple_ptr.ptr(), digits_obj.ptr(), exponent); diff --git a/mssql_python/pybind/py_type_cache.hpp b/mssql_python/pybind/py_type_cache.hpp index 19a9c1f04..df1bb5d68 100644 --- a/mssql_python/pybind/py_type_cache.hpp +++ b/mssql_python/pybind/py_type_cache.hpp @@ -1,4 +1,4 @@ -// py_type_cache.hpp — One-time cache of Python type objects and MONEY boundary constants. +// py_type_cache.hpp — One-time cache of Python type objects. // // Called on first execute(). Uses raw CPython API (not pybind11) because // these cached PyObject* are compared via PyObject_IsInstance in the @@ -24,10 +24,6 @@ inline PyObject* date_class = nullptr; inline PyObject* time_class = nullptr; inline PyObject* decimal_class = nullptr; inline PyObject* uuid_class = nullptr; -inline PyObject* money_min = nullptr; -inline PyObject* money_max = nullptr; -inline PyObject* smallmoney_min = nullptr; -inline PyObject* smallmoney_max = nullptr; inline bool cache_initialized = false; // Import a module and extract an attribute. Returns a new reference. @@ -69,24 +65,12 @@ inline void initialize() { py::object dec_cls = steal(import_attr("decimal", "Decimal")); py::object uuid_cls = steal(import_attr("uuid", "UUID")); - // Pre-compute MONEY/SMALLMONEY boundary Decimals for exact comparison - // in DetectParamTypes (avoids double-precision boundary errors). - py::object sm_min = steal(PyObject_CallFunction(dec_cls.ptr(), "s", "-214748.3648")); - py::object sm_max = steal(PyObject_CallFunction(dec_cls.ptr(), "s", "214748.3647")); - py::object m_min = steal(PyObject_CallFunction(dec_cls.ptr(), "s", "-922337203685477.5808")); - py::object m_max = steal(PyObject_CallFunction(dec_cls.ptr(), "s", "922337203685477.5807")); - if (!sm_min || !sm_max || !m_min || !m_max) throw py::error_already_set(); - // Commit to globals — all acquisitions succeeded. datetime_class = dt_cls.release().ptr(); date_class = date_cls.release().ptr(); time_class = time_cls.release().ptr(); decimal_class = dec_cls.release().ptr(); uuid_class = uuid_cls.release().ptr(); - smallmoney_min = sm_min.release().ptr(); - smallmoney_max = sm_max.release().ptr(); - money_min = m_min.release().ptr(); - money_max = m_max.release().ptr(); cache_initialized = true; } diff --git a/tests/test_020_money_smallmoney.py b/tests/test_020_money_smallmoney.py index 912944c46..9bb0b4004 100644 --- a/tests/test_020_money_smallmoney.py +++ b/tests/test_020_money_smallmoney.py @@ -4,9 +4,10 @@ Validates that Python Decimal values are correctly bound and round-tripped through MONEY, SMALLMONEY, and DECIMAL columns with proper precision handling. -Key implementation detail: MONEY-range Decimals use string binding (SQL_VARCHAR) -because SQL_NUMERIC binding fails with ODBC "Numeric value out of range" error. -String binding preserves full precision and SQL Server handles conversion. +Key implementation detail: every finite Decimal binds as 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). """ import pytest @@ -640,3 +641,119 @@ def test_both_null(cursor, db_connection): finally: drop_table_if_exists(cursor, table_name) db_connection.commit() + + +# ============================================================================= +# GH-740: in-range Decimal must bind as SQL_NUMERIC, not VARCHAR +# ============================================================================= + + +def test_gh740_in_range_decimal_numeric_comparison_no_overflow(cursor, db_connection): + """A money-range Decimal compared against a smaller numeric column must not raise. + + Before the fix the value was bound as VARCHAR, so SQL Server did a + varchar->numeric conversion that overflowed instead of simply not matching. + """ + table_name = "#pytest_gh740_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() + + # Both probes sit inside the MONEY range but exceed numeric(5,2); they must + # return no rows rather than overflow. + cursor.execute(f"SELECT COUNT(*) FROM {table_name} WHERE v = ?", [Decimal("12345.6789")]) + assert cursor.fetchone()[0] == 0 + cursor.execute(f"SELECT COUNT(*) FROM {table_name} WHERE v = ?", [Decimal("300000.00")]) + assert cursor.fetchone()[0] == 0 + # The matching value still matches. + 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_gh740_numeric_param_not_in_first_position(cursor, db_connection): + """A SQL_NUMERIC parameter in any position (not just the first) must bind correctly. + + Guards the descriptor-record fix: the APD record number was hardcoded to 1, so a + numeric param after a NULL (or any earlier param) was written onto the wrong record + and the driver raised "Numeric value out of range". + """ + table_name = "#pytest_gh740_pos" + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (a int, b numeric(6,4))") + cursor.execute(f"INSERT INTO {table_name} VALUES (?, ?)", [None, Decimal("67.8900")]) + db_connection.commit() + + cursor.execute(f"SELECT a, b FROM {table_name}") + row = cursor.fetchone() + assert row[0] is None + assert row[1] == Decimal("67.8900") + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit() + + +def test_gh740_multiple_numerics_with_null_between(cursor, db_connection): + """Multiple SQL_NUMERIC params with differing scales and a NULL between them.""" + table_name = "#pytest_gh740_multi" + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (a numeric(10,4), b int, c numeric(8,2))") + cursor.execute( + f"INSERT INTO {table_name} VALUES (?, ?, ?)", + [Decimal("1.2300"), None, Decimal("999999.99")], + ) + db_connection.commit() + + cursor.execute(f"SELECT a, b, c FROM {table_name}") + row = cursor.fetchone() + assert row[0] == Decimal("1.2300") + assert row[1] is None + assert row[2] == Decimal("999999.99") + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit() + + +def test_gh740_money_boundary_still_round_trips(cursor, db_connection): + """MONEY/SMALLMONEY boundary values still insert exactly after the binding change.""" + for coltype, value in [ + ("MONEY", Decimal("922337203685477.5807")), + ("MONEY", Decimal("-922337203685477.5808")), + ("SMALLMONEY", Decimal("214748.3647")), + ("SMALLMONEY", Decimal("-214748.3648")), + ]: + table_name = "#pytest_gh740_bound" + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (v {coltype})") + cursor.execute(f"INSERT INTO {table_name} VALUES (?)", [value]) + db_connection.commit() + cursor.execute(f"SELECT v FROM {table_name}") + assert cursor.fetchone()[0] == value + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit() + + +def test_gh740_same_statement_changing_precision(cursor, db_connection): + """Re-executing the same statement with Decimals of different precision/scale works.""" + table_name = "#pytest_gh740_reexec" + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (v numeric(20,6))") + for value in [Decimal("1.5"), Decimal("123456.789012"), Decimal("0.000001")]: + cursor.execute(f"INSERT INTO {table_name} VALUES (?)", [value]) + db_connection.commit() + + cursor.execute(f"SELECT v FROM {table_name} ORDER BY v") + rows = [r[0] for r in cursor.fetchall()] + assert rows == [Decimal("0.000001"), Decimal("1.500000"), Decimal("123456.789012")] + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit() From bb77e1ed5400ec5674a27ac8194a323839471cb8 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:24:14 +0530 Subject: [PATCH 2/6] DOC: scope test_020 docstring to the execute() path Clarify that the always-SQL_NUMERIC binding applies to execute(); executemany still string-binds Decimals (GH-503). (GH-740) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_020_money_smallmoney.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_020_money_smallmoney.py b/tests/test_020_money_smallmoney.py index 9bb0b4004..9f0bd262e 100644 --- a/tests/test_020_money_smallmoney.py +++ b/tests/test_020_money_smallmoney.py @@ -4,10 +4,12 @@ Validates that Python Decimal values are correctly bound and round-tripped through MONEY, SMALLMONEY, and DECIMAL columns with proper precision handling. -Key implementation detail: every finite Decimal binds as 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). +Key implementation detail: on the execute() path every finite Decimal binds as +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. """ import pytest From c2e1e36cb66e6ff7502daaa29219cd2bf0a250d4 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:18:02 +0530 Subject: [PATCH 3/6] CHORE: assert native money-range Decimal binds as numeric in parity test test_money_range_decimal_binds_wide only round-tripped the value, so it stayed green after the native C type changed to NUMERIC. Assert the declared base type via sql_variant, and note that _map_sql_type still text-binds money-range Decimals to protect executemany's string binding. (GH-740) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_023_execute_path_parity.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/test_023_execute_path_parity.py b/tests/test_023_execute_path_parity.py index 65f533aa4..9fdafcb94 100644 --- a/tests/test_023_execute_path_parity.py +++ b/tests/test_023_execute_path_parity.py @@ -513,9 +513,16 @@ def test_time_param_binds_wide(cursor): def test_money_range_decimal_binds_wide(cursor): - """Decimals inside the MONEY range are formatted to text and bound with the text - C type, the third consumer of the platform-dependent constant.""" + """On the native path a money-range Decimal now binds as NUMERIC (GH-740), not text. + + This asserts the declared base type via sql_variant, not just a value round-trip, + so a wrong-but-convertible C type cannot pass silently. Note the native path here + intentionally diverges from ``_map_sql_type`` (which still text-binds money-range + Decimals to protect executemany's string binding); see + ``test_map_sql_type_money_range_binds_as_text``. + """ value = decimal.Decimal("214748.3647") + assert _param_basetype(cursor, value) == "numeric" cursor.execute("SELECT CAST(? AS MONEY)", [value]) assert cursor.fetchone()[0] == value @@ -730,7 +737,13 @@ def test_map_sql_type_aware_datetime(cursor): ) def test_map_sql_type_money_range_binds_as_text(cursor, value): """MONEY / SMALLMONEY range Decimals are formatted to text and the slot is - replaced with that formatted string.""" + replaced with that formatted string. + + This is the Python reference path (legacy execute via setinputsizes, and + executemany). It intentionally diverges from the native path, which binds these + as NUMERIC after GH-740; the text binding is retained here because executemany + string-binds Decimals and relies on the server to coerce mixed-scale batches. + """ params = [value] sql_type, c_type, column_size, decimal_digits, is_dae = cursor._map_sql_type(value, params, 0) assert (sql_type, c_type, decimal_digits, is_dae) == ( From 90d43045e39eeac4c5e42e1d23a1e15dd0c08868 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:51:56 +0530 Subject: [PATCH 4/6] CHORE: pin collateral param corruption in GH-740 descriptor test The position test used a NULL first param, which masked the old record-1 bug (record 1 held no data). Use a non-null value first and assert it round-trips intact, so the test pins the collateral corruption of the earlier parameter, not just the numeric's own misplacement. Verified it fails against the pre-fix binder. (GH-740) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_020_money_smallmoney.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/test_020_money_smallmoney.py b/tests/test_020_money_smallmoney.py index 9f0bd262e..3846f8ee2 100644 --- a/tests/test_020_money_smallmoney.py +++ b/tests/test_020_money_smallmoney.py @@ -678,23 +678,29 @@ def test_gh740_in_range_decimal_numeric_comparison_no_overflow(cursor, db_connec def test_gh740_numeric_param_not_in_first_position(cursor, db_connection): - """A SQL_NUMERIC parameter in any position (not just the first) must bind correctly. + """A numeric param at position 2+ must not corrupt the parameter bound before it. Guards the descriptor-record fix: the APD record number was hardcoded to 1, so a - numeric param after a NULL (or any earlier param) was written onto the wrong record - and the driver raised "Numeric value out of range". + numeric param at any later position wrote its type/precision/scale/data-ptr onto + record 1, clobbering the FIRST parameter's binding as collateral. Putting a non-null + value first pins that collateral corruption - the first value must round-trip intact, + not just the numeric's own value. A NULL first would mask it (record 1 held no data). """ table_name = "#pytest_gh740_pos" try: drop_table_if_exists(cursor, table_name) - cursor.execute(f"CREATE TABLE {table_name} (a int, b numeric(6,4))") - cursor.execute(f"INSERT INTO {table_name} VALUES (?, ?)", [None, Decimal("67.8900")]) + cursor.execute(f"CREATE TABLE {table_name} (a int, b varchar(10), c numeric(6,4))") + cursor.execute( + f"INSERT INTO {table_name} VALUES (?, ?, ?)", + [12345, "keep", Decimal("67.8900")], + ) db_connection.commit() - cursor.execute(f"SELECT a, b FROM {table_name}") + cursor.execute(f"SELECT a, b, c FROM {table_name}") row = cursor.fetchone() - assert row[0] is None - assert row[1] == Decimal("67.8900") + assert row[0] == 12345 # first param intact despite the later numeric + assert row[1] == "keep" + assert row[2] == Decimal("67.8900") finally: drop_table_if_exists(cursor, table_name) db_connection.commit() From 374ec85adc36cb95ab7da06c9f4e076cf358b14a Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:07:20 +0530 Subject: [PATCH 5/6] FIX: bind money-range Decimal as numeric on the legacy execute path too The native execute() path was fixed for GH-740, but a real execute() still reaches the Python _map_sql_type when setinputsizes() covers fewer positions than parameters: the uncovered money-range Decimal took the VARCHAR shortcut and overflowed a numeric comparison. Thread a decimal_as_numeric flag so the legacy execute path binds every finite Decimal as SQL_NUMERIC, while executemany keeps its batch VARCHAR string binding (GH-503) unchanged. Adds a partial-setinputsizes regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/cursor.py | 32 ++++++++++++++++++++++----- tests/test_023_execute_path_parity.py | 26 ++++++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index f9abaa506..02915a952 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -650,6 +650,7 @@ def _map_sql_type( # pylint: disable=too-many-arguments,too-many-positional-arg i: int, min_val: Optional[Any] = None, max_val: Optional[Any] = None, + decimal_as_numeric: bool = False, ) -> Tuple[int, int, int, int, bool]: """ Map a Python data type to the corresponding SQL type, @@ -658,6 +659,11 @@ def _map_sql_type( # pylint: disable=too-many-arguments,too-many-positional-arg - param: The parameter to map. - parameters_list: The list of parameters to bind. - 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). Returns: - A tuple containing the SQL type, C type, column size, and decimal digits. """ @@ -793,8 +799,12 @@ 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 - if SMALLMONEY_MIN <= param <= SMALLMONEY_MAX: + # 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). + if not decimal_as_numeric and SMALLMONEY_MIN <= param <= SMALLMONEY_MAX: logger.debug("_map_sql_type: DECIMAL -> SMALLMONEY - index=%d", i) # smallmoney parameters_list[i] = format(param, "f") @@ -805,7 +815,7 @@ def _map_sql_type( # pylint: disable=too-many-arguments,too-many-positional-arg 0, False, ) - if MONEY_MIN <= param <= MONEY_MAX: + if not decimal_as_numeric and MONEY_MIN <= param <= MONEY_MAX: logger.debug("_map_sql_type: DECIMAL -> MONEY - index=%d", i) # money parameters_list[i] = format(param, "f") @@ -1247,6 +1257,7 @@ def _create_parameter_types_list( # pylint: disable=too-many-arguments,too-many i: int, min_val: Optional[Any] = None, max_val: Optional[Any] = None, + decimal_as_numeric: bool = False, ) -> Tuple[int, int, int, int, bool]: """ Maps parameter types for the given parameter. @@ -1313,7 +1324,12 @@ def _create_parameter_types_list( # pylint: disable=too-many-arguments,too-many else: # Fall back to automatic type inference sql_type, c_type, column_size, decimal_digits, is_dae = self._map_sql_type( - parameter, parameters_list, i, min_val=min_val, max_val=max_val + parameter, + parameters_list, + i, + min_val=min_val, + max_val=max_val, + decimal_as_numeric=decimal_as_numeric, ) # If TIME values are being bound via text C-types, normalize them to a @@ -1827,7 +1843,13 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state if parameters: param_info = ddbc_bindings.ParamInfo for i, param in enumerate(parameters): - paraminfo = self._create_parameter_types_list(param, param_info, parameters, i) + # 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). + paraminfo = self._create_parameter_types_list( + param, param_info, parameters, i, decimal_as_numeric=True + ) parameters_type.append(paraminfo) if logger.isEnabledFor(logging.DEBUG): diff --git a/tests/test_023_execute_path_parity.py b/tests/test_023_execute_path_parity.py index 9fdafcb94..4813bc3e7 100644 --- a/tests/test_023_execute_path_parity.py +++ b/tests/test_023_execute_path_parity.py @@ -292,6 +292,32 @@ def test_setinputsizes_shorter_than_params_detects_the_rest(cursor): cursor.setinputsizes(None) +def test_setinputsizes_shorter_than_params_money_decimal_binds_numeric(cursor): + """GH-740 on the legacy path: an uncovered money-range Decimal must bind as + NUMERIC, not VARCHAR. + + With setinputsizes shorter than the parameter list, the uncovered Decimal falls + through to _map_sql_type. Before the fix it took the MONEY-range VARCHAR shortcut, + so a comparison against a smaller numeric column made SQL Server convert + varchar->numeric and overflow. It must now return no match without raising, like + the native path. + """ + cursor.execute("DROP TABLE IF EXISTS #pytest_si_gh740") + cursor.execute("CREATE TABLE #pytest_si_gh740 (k int, v numeric(5,2))") + cursor.execute("INSERT INTO #pytest_si_gh740 VALUES (1, 12.34)") + cursor.setinputsizes([(ddbc_sql_const.SQL_INTEGER.value, 0, 0)]) # sizes param 0 only + try: + with pytest.warns(Warning): # count mismatch is warned, then execution proceeds + cursor.execute( + "SELECT COUNT(*) FROM #pytest_si_gh740 WHERE k = ? AND v = ?", + [1, decimal.Decimal("12345.6789")], + ) + assert cursor.fetchone()[0] == 0 + finally: + cursor.setinputsizes(None) + cursor.execute("DROP TABLE IF EXISTS #pytest_si_gh740") + + # --------------------------------------------------------------------------- # Edge case tests (issues caught in rubber-duck review) # --------------------------------------------------------------------------- From b3dcaad0e9ab3163b03693fa97fa7518c7388be7 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:32:44 +0530 Subject: [PATCH 6/6] CHORE: changelog entry and edge-case tests for GH-740 binding change Document the money-range Decimal binding change (now numeric, not varchar) in CHANGELOG, including the on-the-wire behavioral effects (SELECT ? returns Decimal, sql_variant base type, and numeric type-precedence causing CONVERT_IMPLICIT on the column side). Add two edge-case tests: an exact numeric(38,38) round-trip of 1E-38 asserted via as_tuple() (loose-tolerance coverage elsewhere would miss a silent zero), and signed-zero normalization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 17 +++++++++++ tests/test_020_money_smallmoney.py | 47 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ec2ae5c6..28e2469db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,23 @@ 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-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 + `setinputsizes()` covers fewer positions than parameters). Previously it was + bound as a formatted `VARCHAR` based on the value alone, so comparing it against + a smaller `numeric`/`decimal` column (`WHERE v = ?`) made SQL Server convert + `varchar`→`numeric` and raise an arithmetic overflow instead of simply not + matching. Also fixes a latent binder bug the shortcut was masking: the numeric + APD descriptor record number was hardcoded to `1`, so a numeric parameter in any + position other than the first corrupted the parameter bound at position 1. + **Behavioral change on the wire:** money-range Decimals now arrive as `numeric` + rather than `varchar` — a bare `SELECT ?` returns `Decimal` instead of `str`, + `sql_variant` stores them as `numeric`, and because `numeric` outranks + `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. - **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/tests/test_020_money_smallmoney.py b/tests/test_020_money_smallmoney.py index 3846f8ee2..f60d37e00 100644 --- a/tests/test_020_money_smallmoney.py +++ b/tests/test_020_money_smallmoney.py @@ -765,3 +765,50 @@ def test_gh740_same_statement_changing_precision(cursor, db_connection): finally: drop_table_if_exists(cursor, table_name) db_connection.commit() + + +def test_gh740_smallest_scale_38_exact_roundtrip(cursor, db_connection): + """numeric(38,38) round-trip of Decimal("1E-38") must be exact, not a silent 0. + + Asserts via as_tuple() rather than == or a tolerance: a mantissa bug in the + numeric binding that dropped the value to 0 would still satisfy a loose + approx-style check, so pin the exact digits and exponent. + """ + table_name = "#pytest_gh740_tiny" + value = Decimal("1E-38") + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (v numeric(38,38))") + cursor.execute(f"INSERT INTO {table_name} VALUES (?)", [value]) + db_connection.commit() + + cursor.execute(f"SELECT v FROM {table_name}") + got = cursor.fetchone()[0] + assert got != Decimal(0) + assert got.as_tuple() == value.as_tuple() + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit() + + +def test_gh740_signed_zero_normalizes(cursor, db_connection): + """Signed-zero Decimals bind as NUMERIC and come back as unsigned zero. + + NUMERIC has no negative zero, so Decimal("-0")/"-0.00" normalize to positive + zero on the round-trip. This documents the behavior the bind-type switch makes + explicit (the old VARCHAR path also normalized server-side). + """ + table_name = "#pytest_gh740_negzero" + try: + drop_table_if_exists(cursor, table_name) + cursor.execute(f"CREATE TABLE {table_name} (v numeric(10,2))") + cursor.execute(f"INSERT INTO {table_name} VALUES (?)", [Decimal("-0.00")]) + db_connection.commit() + + cursor.execute(f"SELECT v FROM {table_name}") + got = cursor.fetchone()[0] + assert got == Decimal("0.00") + assert got.as_tuple().sign == 0 # normalized to unsigned zero + finally: + drop_table_if_exists(cursor, table_name) + db_connection.commit()