Skip to content
Open
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
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ 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. SQL precision/scale stay on `columnSize` /
`decimalDigits`; the CHAR array stride uses a separate `bufferSize`
sized from the longest fixed-point encoding (e.g. `Decimal("1E-38")`),
so near-max precision values are not rejected by the array buffer.
The `setinputsizes` DECIMAL/NUMERIC string path uses the same buffer-width
split; buffer width is derived from text produced by the protected
conversion path so failed conversions still raise a sanitized `ValueError`
with row/column details (no raw MemoryError/RuntimeError leakage).
- **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
Expand Down
134 changes: 123 additions & 11 deletions mssql_python/cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,9 +666,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.
"""
Expand Down Expand Up @@ -804,11 +804,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
Expand Down Expand Up @@ -1794,6 +1794,7 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state
)
else:
ret = ddbc_bindings.DDBCSQLExecDirect(self.hstmt, operation)

# Check return code
try:

Expand Down Expand Up @@ -2287,6 +2288,53 @@ 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.

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
found = False
for value in column:
if not isinstance(value, decimal.Decimal):
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)
Comment on lines +2326 to +2333
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.
Expand Down Expand Up @@ -2500,12 +2548,25 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s
is_dae = True

# Sanitize precision/scale for numeric types
numeric_buffer_size = 0
if sql_type in (
ddbc_sql_const.SQL_DECIMAL.value,
ddbc_sql_const.SQL_NUMERIC.value,
):
column_size = max(1, min(int(column_size) if column_size > 0 else 18, 38))
decimal_digits = min(max(0, decimal_digits), column_size)
# Provisional SQL_C_CHAR stride: size only from values that
# are already Decimal. Do NOT convert non-Decimals here —
# that bypasses the protected conversion loop below and can
# leak MemoryError/RuntimeError (and value-bearing messages).
# After conversion, bufferSize is widened from the produced
# fixed-point text (same path that sanitizes failures).
max_encoded = 0
for row in seq_of_parameters:
value = row[col_index]
if isinstance(value, decimal.Decimal):
max_encoded = max(max_encoded, len(format(value, "f")))
numeric_buffer_size = max(max_encoded, column_size + 3, 1)

# For binary data columns with mixed content, we need to find max size
if sql_type in (
Expand Down Expand Up @@ -2536,6 +2597,8 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s
paraminfo.columnSize = column_size
paraminfo.decimalDigits = decimal_digits
paraminfo.isDAE = is_dae
if numeric_buffer_size:
paraminfo.bufferSize = numeric_buffer_size

# Ensure we never have SQL_C_DEFAULT (0) for C-type
if paraminfo.paramCType == 0:
Expand All @@ -2551,6 +2614,18 @@ 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.
# 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(
sample_value,
Expand All @@ -2559,6 +2634,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++,
Expand All @@ -2576,9 +2652,26 @@ 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
if max_decimal_len > paraminfo.columnSize:
paraminfo.columnSize = max_decimal_len
# 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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this uses sql numeric precision as the array's character-buffer width too. precision only counts digits, but the fixed-point string also needs space for the sign, decimal point, leading zero, and terminator. Decimal("1E-38") gets (38, 38) here but formats to 40 characters, so PyStringArrayDataBuffer rejects it before sql server sees it.

I ran the same cases on this head and the exact merge base. Decimal("1E-38") and [-12.34, 56.78] both complete on the base; this head raises RuntimeError: Input string exceeds allowed column size at parameter index 0. the linux and windows jobs show the same 13 decimal failures too.

can we keep sql precision and scale separate from the SQL_C_CHAR buffer stride, sizing the buffer from the longest encoded fixed-point value plus its terminator?

batch_precision, batch_scale = self._batch_decimal_precision_scale(column)

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.

batch_precision is applied to columnSize without a <=38 check. The sample-level guard in _map_sql_type doesn't cover the whole batch, so a mixed batch (many integer digits in one row, many fractional in another) can produce batch_precision > 38 even when each value is individually storable, and the driver then fails with an opaque message. Consider raising the existing "precision too high" ValueError when batch_precision > 38. Don't clamp, since that would silently truncate.

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
# SQL_C_CHAR array stride is separate from SQL precision.
# Fixed-point strings need room for sign, '.', and a leading
# zero (e.g. Decimal("1E-38") -> 40 chars with precision 38).
# Size from the longest encoded value in the batch.
paraminfo.bufferSize = max(max_decimal_len, 1)

# Correct column size for Decimal columns sent as SQL_VARCHAR (GH-557).
# The sample value's formatted string may be shorter than another
Expand Down Expand Up @@ -2678,6 +2771,25 @@ def executemany( # pylint: disable=too-many-locals,too-many-branches,too-many-s
raise ValueError(err_msg) from None
processed_parameters.append(processed_row)

# Derive/widen SQL_C_CHAR bufferSize from text produced by the protected
# DECIMAL/NUMERIC conversion above. setinputsizes previously sized by
# converting independently (leaking raw MemoryError/RuntimeError); the
# auto-detect path already had a Decimal-only provisional size.
for col_index, ptype in enumerate(parameters_type):
if ptype.paramSQLType not in (
ddbc_sql_const.SQL_DECIMAL.value,
ddbc_sql_const.SQL_NUMERIC.value,
):
continue
max_encoded = 0
for row in processed_parameters:
val = row[col_index]
if isinstance(val, str):
max_encoded = max(max_encoded, len(val))
if max_encoded:
prior = getattr(ptype, "bufferSize", 0) or 0
ptype.bufferSize = max(prior, max_encoded, 1)

# Now transpose the processed parameters
columnwise_params, row_count = self._transpose_rowwise_to_columnwise(processed_parameters)

Expand Down
Loading