Skip to content

Commit adda208

Browse files
committed
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.)
1 parent 93ef1bd commit adda208

2 files changed

Lines changed: 31 additions & 2 deletions

File tree

‎mssql_python/cursor.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2390,7 +2390,13 @@ def _apply_money_batch_declared_type(self, paraminfo, column):
23902390
non_nulls = [v for v in column if v is not None]
23912391
if not non_nulls or not all(isinstance(v, decimal.Decimal) for v in non_nulls):
23922392
return False
2393-
if not all(SMALLMONEY_MIN <= v <= MONEY_MAX for v in non_nulls):
2393+
# The money shortcut (see _map_sql_type) string-binds any Decimal in
2394+
# the full MONEY range; SMALLMONEY is a subset of that range. The
2395+
# lower bound must be MONEY_MIN, not SMALLMONEY_MIN: a value like
2396+
# Decimal("-300000") is in the MONEY range but below SMALLMONEY_MIN,
2397+
# and using the smaller floor left such batches as SQL_VARCHAR,
2398+
# reintroducing the GH-745 overflow on negative money-range values.
2399+
if not all(MONEY_MIN <= v <= MONEY_MAX for v in non_nulls):
23942400
return False
23952401
paraminfo.paramSQLType = ddbc_sql_const.SQL_NUMERIC.value
23962402
paraminfo.decimalDigits = max(

‎tests/test_020_money_smallmoney.py‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,15 @@ def test_executemany_money_smallmoney(cursor, db_connection):
530530

531531

532532
class _ParamInfo:
533-
"""Duck-typed stand-in for ddbc_bindings.ParamInfo (no C extension needed)."""
533+
"""Duck-typed stand-in for ddbc_bindings.ParamInfo.
534+
535+
It avoids *constructing* a real ParamInfo (the pure-Python fields
536+
`_apply_money_batch_declared_type` reads are enough), but the
537+
`Cursor` import in `_bare_cursor` still pulls in the compiled
538+
ddbc_bindings extension, so the test runs only where it is built
539+
(CI); arm64 local runs stub the module (see the GH-745 red/green
540+
script) rather than this file.
541+
"""
534542

535543
def __init__(self, sql_type):
536544
self.paramSQLType = sql_type
@@ -570,6 +578,21 @@ def test_money_batch_redeclared_as_numeric():
570578
assert p.paramSQLType == numeric
571579
assert p.decimalDigits == 0
572580

581+
# Negative money-range value below SMALLMONEY_MIN but inside the MONEY
582+
# range: must still redeclare (the MONEY_MIN floor, not the SMALLMONEY
583+
# floor, is the correct lower bound). GH-745 negative-value edge.
584+
p = _ParamInfo(varchar)
585+
assert cur._apply_money_batch_declared_type(p, [Decimal("-300000")]) is True
586+
assert p.paramSQLType == numeric
587+
assert p.decimalDigits == 0
588+
589+
# Below MONEY_MIN (negative overflow) -> unchanged (not a money value)
590+
p = _ParamInfo(varchar)
591+
assert cur._apply_money_batch_declared_type(
592+
p, [Decimal("-922337203685477.5809")]
593+
) is False
594+
assert p.paramSQLType == varchar
595+
573596
# Above MONEY_MAX -> unchanged (existing NUMERIC path handles it)
574597
p = _ParamInfo(varchar)
575598
assert cur._apply_money_batch_declared_type(p, [Decimal("9999999999999999")]) is False

0 commit comments

Comments
 (0)