Skip to content

Commit cd828b6

Browse files
authored
FIX: Fix precision loss when binding large Decimal values to SQL_NUMERIC_STRUCT (#287)
### Work Item / Issue Reference <!-- IMPORTANT: Please follow the PR template guidelines below. For mssql-python maintainers: Insert your ADO Work Item ID below (e.g. AB#37452) For external contributors: Insert Github Issue number below (e.g. #149) Only one reference is required - either GitHub issue OR ADO Work Item. --> <!-- mssql-python maintainers: ADO Work Item --> > [AB#38111](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/38111) <!-- External contributors: GitHub Issue --> > GitHub Issue: #<ISSUE_NUMBER> ------------------------------------------------------------------- ### Summary <!-- Insert your summary of changes below. Minimum 10 characters required. --> This pull request significantly improves the handling of SQL Server NUMERIC/DECIMAL values in both the Python and C++ layers, addressing precision, scale, and binary representation for high-precision decimals. It also introduces a comprehensive suite of tests to validate numeric roundtrip, edge cases, and boundary conditions. The changes ensure compliance with SQL Server's maximum precision (38 digits), robust conversion between Python decimals and SQL binary formats, and better test coverage for numeric types. ### Numeric Data Handling Improvements * The `_get_numeric_data` method in `cursor.py` now correctly calculates the binary representation of decimal values, supporting up to 38 digits of precision, and constructs the byte array for SQL Server compatibility. The restriction on precision is raised from 15 to 38 digits. [[1]](diffhunk://#diff-deceea46ae01082ce8400e14fa02f4b7585afb7b5ed9885338b66494f5f38280L198-R199) [[2]](diffhunk://#diff-deceea46ae01082ce8400e14fa02f4b7585afb7b5ed9885338b66494f5f38280L218-R223) [[3]](diffhunk://#diff-deceea46ae01082ce8400e14fa02f4b7585afb7b5ed9885338b66494f5f38280L232-R251) * The C++ `NumericData` struct now stores the value as a binary string (16 bytes) instead of a 64-bit integer, allowing support for high-precision numerics. Related memory handling is updated for parameter binding. [[1]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1R24) [[2]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L59-R65) [[3]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L560-R564) [[4]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L2055-R2065) [[5]](diffhunk://#diff-dde2297345718ec449a14e7dff91b7bb2342b008ecc071f562233646d71144a1L3797-R3801) ### Test Suite Expansion and Refactoring * Old numeric tests were removed and replaced with a new, thorough set of tests covering roundtrip for basic, high-precision, negative, zero, small, boundary, NULL, fetchmany, and executemany scenarios for numeric values. This ensures that all critical cases are validated. [[1]](diffhunk://#diff-82594712308ff34afa8b067af67db231e9a1372ef474da3db121e14e4d418f69L1643-L1658) [[2]](diffhunk://#diff-82594712308ff34afa8b067af67db231e9a1372ef474da3db121e14e4d418f69L1724-L1765) [[3]](diffhunk://#diff-82594712308ff34afa8b067af67db231e9a1372ef474da3db121e14e4d418f69R11348-R11564) --- These changes collectively make the library more robust and compliant with SQL Server's numeric type requirements, and the expanded tests will help catch future regressions. <!-- ### PR Title Guide > For feature requests FEAT: (short-description) > For non-feature requests like test case updates, config updates , dependency updates etc CHORE: (short-description) > For Fix requests FIX: (short-description) > For doc update requests DOC: (short-description) > For Formatting, indentation, or styling update STYLE: (short-description) > For Refactor, without any feature changes REFACTOR: (short-description) > For release related changes, without any feature changes RELEASE: #<RELEASE_VERSION> (short-description) ### Contribution Guidelines External contributors: - Create a GitHub issue first: https://github.com/microsoft/mssql-python/issues/new - Link the GitHub issue in the "GitHub Issue" section above - Follow the PR title format and provide a meaningful summary mssql-python maintainers: - Create an ADO Work Item following internal processes - Link the ADO Work Item in the "ADO Work Item" section above - Follow the PR title format and provide a meaningful summary -->
1 parent 8a508b6 commit cd828b6

File tree

3 files changed

+453
-86
lines changed

3 files changed

+453
-86
lines changed

mssql_python/cursor.py

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ def _get_numeric_data(self, param):
195195
the numeric data.
196196
"""
197197
decimal_as_tuple = param.as_tuple()
198-
num_digits = len(decimal_as_tuple.digits)
198+
digits_tuple = decimal_as_tuple.digits
199+
num_digits = len(digits_tuple)
199200
exponent = decimal_as_tuple.exponent
200201

201202
# Calculate the SQL precision & scale
@@ -215,12 +216,11 @@ def _get_numeric_data(self, param):
215216
precision = exponent * -1
216217
scale = exponent * -1
217218

218-
# TODO: Revisit this check, do we want this restriction?
219-
if precision > 15:
219+
if precision > 38:
220220
raise ValueError(
221221
"Precision of the numeric value is too high - "
222222
+ str(param)
223-
+ ". Should be less than or equal to 15"
223+
+ ". Should be less than or equal to 38"
224224
)
225225
Numeric_Data = ddbc_bindings.NumericData
226226
numeric_data = Numeric_Data()
@@ -229,12 +229,26 @@ def _get_numeric_data(self, param):
229229
numeric_data.sign = 1 if decimal_as_tuple.sign == 0 else 0
230230
# strip decimal point from param & convert the significant digits to integer
231231
# Ex: 12.34 ---> 1234
232-
val = str(param)
233-
if "." in val or "-" in val:
234-
val = val.replace(".", "")
235-
val = val.replace("-", "")
236-
val = int(val)
237-
numeric_data.val = val
232+
int_str = ''.join(str(d) for d in digits_tuple)
233+
if exponent > 0:
234+
int_str = int_str + ('0' * exponent)
235+
elif exponent < 0:
236+
if -exponent > num_digits:
237+
int_str = ('0' * (-exponent - num_digits)) + int_str
238+
239+
if int_str == '':
240+
int_str = '0'
241+
242+
# Convert decimal base-10 string to python int, then to 16 little-endian bytes
243+
big_int = int(int_str)
244+
byte_array = bytearray(16) # SQL_MAX_NUMERIC_LEN
245+
for i in range(16):
246+
byte_array[i] = big_int & 0xFF
247+
big_int >>= 8
248+
if big_int == 0:
249+
break
250+
251+
numeric_data.val = bytes(byte_array)
238252
return numeric_data
239253

240254
def _map_sql_type(self, param, parameters_list, i, min_val=None, max_val=None):
@@ -307,7 +321,27 @@ def _map_sql_type(self, param, parameters_list, i, min_val=None, max_val=None):
307321
)
308322

309323
if isinstance(param, decimal.Decimal):
310-
# Detect MONEY / SMALLMONEY range
324+
# First check precision limit for all decimal values
325+
decimal_as_tuple = param.as_tuple()
326+
digits_tuple = decimal_as_tuple.digits
327+
num_digits = len(digits_tuple)
328+
exponent = decimal_as_tuple.exponent
329+
330+
# Calculate the SQL precision (same logic as _get_numeric_data)
331+
if exponent >= 0:
332+
precision = num_digits + exponent
333+
elif (-1 * exponent) <= num_digits:
334+
precision = num_digits
335+
else:
336+
precision = exponent * -1
337+
338+
if precision > 38:
339+
raise ValueError(
340+
f"Precision of the numeric value is too high. "
341+
f"The maximum precision supported by SQL Server is 38, but got {precision}."
342+
)
343+
344+
# Detect MONEY / SMALLMONEY range
311345
if SMALLMONEY_MIN <= param <= SMALLMONEY_MAX:
312346
# smallmoney
313347
parameters_list[i] = str(param)

mssql_python/pybind/ddbc_bindings.cpp

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#define SQL_SS_TIMESTAMPOFFSET (-155)
2222
#define SQL_C_SS_TIMESTAMPOFFSET (0x4001)
2323
#define MAX_DIGITS_IN_NUMERIC 64
24+
#define SQL_MAX_NUMERIC_LEN 16
2425
#define SQL_SS_XML (-152)
2526

2627
#define STRINGIFY_FOR_CASE(x) \
@@ -57,12 +58,18 @@ struct NumericData {
5758
SQLCHAR precision;
5859
SQLSCHAR scale;
5960
SQLCHAR sign; // 1=pos, 0=neg
60-
std::uint64_t val; // 123.45 -> 12345
61+
std::string val; // 123.45 -> 12345
6162

62-
NumericData() : precision(0), scale(0), sign(0), val(0) {}
63+
NumericData() : precision(0), scale(0), sign(0), val(SQL_MAX_NUMERIC_LEN, '\0') {}
6364

64-
NumericData(SQLCHAR precision, SQLSCHAR scale, SQLCHAR sign, std::uint64_t value)
65-
: precision(precision), scale(scale), sign(sign), val(value) {}
65+
NumericData(SQLCHAR precision, SQLSCHAR scale, SQLCHAR sign, const std::string& valueBytes)
66+
: precision(precision), scale(scale), sign(sign), val(SQL_MAX_NUMERIC_LEN, '\0') {
67+
if (valueBytes.size() > SQL_MAX_NUMERIC_LEN) {
68+
throw std::runtime_error("NumericData valueBytes size exceeds SQL_MAX_NUMERIC_LEN (16)");
69+
}
70+
// Copy binary data to buffer, remaining bytes stay zero-padded
71+
std::memcpy(&val[0], valueBytes.data(), valueBytes.size());
72+
}
6673
};
6774

6875
// Struct to hold the DateTimeOffset structure
@@ -558,9 +565,10 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
558565
decimalPtr->sign = decimalParam.sign;
559566
// Convert the integer decimalParam.val to char array
560567
std::memset(static_cast<void*>(decimalPtr->val), 0, sizeof(decimalPtr->val));
561-
std::memcpy(static_cast<void*>(decimalPtr->val),
562-
reinterpret_cast<char*>(&decimalParam.val),
563-
sizeof(decimalParam.val));
568+
size_t copyLen = std::min(decimalParam.val.size(), sizeof(decimalPtr->val));
569+
if (copyLen > 0) {
570+
std::memcpy(decimalPtr->val, decimalParam.val.data(), copyLen);
571+
}
564572
dataPtr = static_cast<void*>(decimalPtr);
565573
break;
566574
}
@@ -2051,15 +2059,17 @@ SQLRETURN BindParameterArray(SQLHANDLE hStmt,
20512059
throw std::runtime_error(MakeParamMismatchErrorStr(info.paramCType, paramIndex));
20522060
}
20532061
NumericData decimalParam = element.cast<NumericData>();
2054-
LOG("Received numeric parameter at [%zu]: precision=%d, scale=%d, sign=%d, val=%lld",
2055-
i, decimalParam.precision, decimalParam.scale, decimalParam.sign, decimalParam.val);
2056-
numericArray[i].precision = decimalParam.precision;
2057-
numericArray[i].scale = decimalParam.scale;
2058-
numericArray[i].sign = decimalParam.sign;
2059-
std::memset(numericArray[i].val, 0, sizeof(numericArray[i].val));
2060-
std::memcpy(numericArray[i].val,
2061-
reinterpret_cast<const char*>(&decimalParam.val),
2062-
std::min(sizeof(decimalParam.val), sizeof(numericArray[i].val)));
2062+
LOG("Received numeric parameter at [%zu]: precision=%d, scale=%d, sign=%d, val=%s",
2063+
i, decimalParam.precision, decimalParam.scale, decimalParam.sign, decimalParam.val.c_str());
2064+
SQL_NUMERIC_STRUCT& target = numericArray[i];
2065+
std::memset(&target, 0, sizeof(SQL_NUMERIC_STRUCT));
2066+
target.precision = decimalParam.precision;
2067+
target.scale = decimalParam.scale;
2068+
target.sign = decimalParam.sign;
2069+
size_t copyLen = std::min(decimalParam.val.size(), sizeof(target.val));
2070+
if (copyLen > 0) {
2071+
std::memcpy(target.val, decimalParam.val.data(), copyLen);
2072+
}
20632073
strLenOrIndArray[i] = sizeof(SQL_NUMERIC_STRUCT);
20642074
}
20652075
dataPtr = numericArray;
@@ -3800,7 +3810,7 @@ PYBIND11_MODULE(ddbc_bindings, m) {
38003810
// Define numeric data class
38013811
py::class_<NumericData>(m, "NumericData")
38023812
.def(py::init<>())
3803-
.def(py::init<SQLCHAR, SQLSCHAR, SQLCHAR, std::uint64_t>())
3813+
.def(py::init<SQLCHAR, SQLSCHAR, SQLCHAR, const std::string&>())
38043814
.def_readwrite("precision", &NumericData::precision)
38053815
.def_readwrite("scale", &NumericData::scale)
38063816
.def_readwrite("sign", &NumericData::sign)

0 commit comments

Comments
 (0)