Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions mssql_python/cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High — derive numeric precision separately from SQL_C_CHAR buffer capacity. This changes the declared SQL type to SQL_NUMERIC, but leaves columnSize as the sample's formatted-string length. BindParameterArray passes that field as SQLBindParameter's ColumnSize, which ODBC defines as precision for SQL_NUMERIC.

For example, [Decimal("9999"), Decimal("0.0001")] selects 0.0001 and becomes NUMERIC(6,4), so 9999 no longer fits. Decimal("1E-38") passes the existing precision-38 check but becomes invalid NUMERIC(40,38) because its formatted text is 40 characters. This is established from the Python inference path and native ODBC arguments; I could not run a live DB reproduction because the cp313-amd64 extension is not built in this worktree.

Please derive a common batch precision as maximum integer digits plus maximum scale, enforce precision <= 38, and carry SQL_C_CHAR text-buffer capacity separately in the native binder. Changing columnSize to precision alone would truncate strings containing a sign, decimal point, or leading zero. Add focused live regressions for the two examples above.

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,
Expand Down Expand Up @@ -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)

Comment on lines +2602 to +2608
# 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
Expand Down
91 changes: 89 additions & 2 deletions tests/test_020_money_smallmoney.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment on lines +569 to +571

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — apply the blocking Black format. The repository's black --check --line-length=100 mssql_python/ tests/ gate reformats this assertion and the analogous below-MONEY_MIN assertion around line 591.

Suggested change
assert cur._apply_money_batch_declared_type(
p, [Decimal("12345.6789"), Decimal("-0.0001"), None]
) is True
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
# =============================================================================
Expand Down