From dcb60f240c6ff6ff81a46ec344fcaff12b9f2389 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 29 Apr 2026 17:16:05 +0530 Subject: [PATCH 01/23] Add C++ DetectParamTypes + SQLExecuteFast pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move parameter type detection from Python into C++ using raw CPython type checks (PyLong_CheckExact, PyFloat_CheckExact, etc.). Merge the DetectParamTypes → BindParameters → SQLExecute pipeline into a single DDBCSQLExecuteFast call so ParamInfo never crosses the pybind11 boundary. - DetectParamTypes: handles int (range-detected), float, bool, str (unicode + geometry sniffing), bytes, datetime/date/time, Decimal (MONEY range + generic numeric), UUID, None, with fallback to string - SQLExecuteFast_wrap: single pipeline with GIL release, always uses SQLPrepare for parameterized queries - cursor.py: fast path routing when no setinputsizes overrides present; old DDBCSQLExecute path preserved for setinputsizes callers - Named constants: MAX_INLINE_CHAR, MAX_INLINE_BINARY, MAX_NUMERIC_PRECISION, MONEY/SMALLMONEY ranges, PARAM_C_TYPE_TEXT platform macro --- mssql_python/cursor.py | 82 +++-- mssql_python/pybind/ddbc_bindings.cpp | 446 ++++++++++++++++++++++++++ 2 files changed, 495 insertions(+), 33 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 05324875..783d0c69 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -1452,11 +1452,6 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state # Getting encoding setting encoding_settings = self._get_encoding_settings() - # Apply timeout if set (non-zero) - logger.debug("execute: Creating parameter type list") - param_info = ddbc_bindings.ParamInfo - parameters_type = [] - # Validate that inputsizes matches parameter count if both are present if parameters and self._inputsizes: if len(self._inputsizes) != len(parameters): @@ -1468,11 +1463,6 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state Warning, ) - if parameters: - for i, param in enumerate(parameters): - paraminfo = self._create_parameter_types_list(param, param_info, parameters, i) - parameters_type.append(paraminfo) - # Prepare caching: skip SQLPrepare when re-executing the same SQL # with parameters. The HSTMT is reused via _soft_reset_cursor, so the # server-side plan from the previous SQLPrepare is still valid. @@ -1481,30 +1471,56 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state self.is_stmt_prepared = [False] effective_use_prepare = use_prepare and not same_sql - if logger.isEnabledFor(logging.DEBUG): - for i, param in enumerate(parameters): - logger.debug( - """Parameter number: %s, Parameter: %s, - Param Python Type: %s, ParamInfo: %s, %s, %s, %s, %s""", - i + 1, - param, - str(type(param)), - parameters_type[i].paramSQLType, - parameters_type[i].paramCType, - parameters_type[i].columnSize, - parameters_type[i].decimalDigits, - parameters_type[i].inputOutputType, - ) - - ret = ddbc_bindings.DDBCSQLExecute( - self.hstmt, - operation, - parameters, - parameters_type, - self.is_stmt_prepared, - effective_use_prepare, - encoding_settings, + # Fast path: when no inputsizes override, do type detection + bind + execute + # entirely in C++. ParamInfo never crosses the pybind11 boundary. + use_fast_path = parameters and not ( + self._inputsizes and any(s is not None for s in self._inputsizes) ) + + if use_fast_path: + ret = ddbc_bindings.DDBCSQLExecuteFast( + self.hstmt, + operation, + parameters, + self.is_stmt_prepared, + effective_use_prepare, + encoding_settings, + ) + else: + # Slow path: Python-side type detection (used when setinputsizes overrides are present) + parameters_type = [] + if parameters: + param_info = ddbc_bindings.ParamInfo + for i, param in enumerate(parameters): + paraminfo = self._create_parameter_types_list( + param, param_info, parameters, i + ) + parameters_type.append(paraminfo) + + if logger.isEnabledFor(logging.DEBUG): + for i, param in enumerate(parameters): + logger.debug( + """Parameter number: %s, Parameter: %s, + Param Python Type: %s, ParamInfo: %s, %s, %s, %s, %s""", + i + 1, + param, + str(type(param)), + parameters_type[i].paramSQLType, + parameters_type[i].paramCType, + parameters_type[i].columnSize, + parameters_type[i].decimalDigits, + parameters_type[i].inputOutputType, + ) + + ret = ddbc_bindings.DDBCSQLExecute( + self.hstmt, + operation, + parameters, + parameters_type, + self.is_stmt_prepared, + effective_use_prepare, + encoding_settings, + ) # Check return code try: diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 9d007653..0fc59701 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -163,6 +163,7 @@ struct ParamInfo { SQLLEN strLenOrInd = 0; // Required for DAE bool isDAE = false; // Indicates if we need to stream py::object dataPtr; + Py_ssize_t utf16Len = 0; // UTF-16 code unit count for string params }; #ifdef __GNUC__ #pragma GCC diagnostic pop @@ -439,6 +440,386 @@ std::string DescribeChar(unsigned char ch) { } } +// --------------------------------------------------------------------------- +// Constants for DetectParamTypes +// --------------------------------------------------------------------------- + +// Strings longer than this use data-at-execution (DAE) streaming +static constexpr int MAX_INLINE_CHAR = 4000; + +// Binary data longer than this uses DAE streaming (SQL Server max for non-MAX types) +static constexpr int MAX_INLINE_BINARY = 8000; + +// SQL Server maximum numeric precision +static constexpr int MAX_NUMERIC_PRECISION = 38; + +// MONEY range: -922,337,203,685,477.5808 to 922,337,203,685,477.5807 +static constexpr double MONEY_MIN = -922337203685477.5808; +static constexpr double MONEY_MAX = 922337203685477.5807; + +// SMALLMONEY range: -214,748.3648 to 214,748.3647 +static constexpr double SMALLMONEY_MIN = -214748.3648; +static constexpr double SMALLMONEY_MAX = 214748.3647; + +// Platform-specific text C type: unixODBC requires all text as wide chars +#if defined(__APPLE__) || defined(__linux__) +static constexpr SQLSMALLINT PARAM_C_TYPE_TEXT = SQL_C_WCHAR; +#else +static constexpr SQLSMALLINT PARAM_C_TYPE_TEXT = SQL_C_CHAR; +#endif + +// Forward declare NumericData helper used by decimal path +static py::object build_numeric_data(const py::object& decimal_param); + +// --------------------------------------------------------------------------- +// DetectParamTypes — C++ type detection for the execute() fast path. +// +// Replaces the Python-side _create_parameter_types_list() loop by doing type +// detection entirely in C++ using raw CPython type checks. +// +// ORDERING MATTERS: +// - bool before int (bool is a subclass of int in Python) +// - datetime before date (datetime is a subclass of date) +// +// Some types mutate the params list in-place via PyList_SET_ITEM: +// - time → normalized to "HH:MM:SS.ffffff" string +// - Decimal in MONEY range → formatted via __format__("f") +// - Decimal (generic) → converted to NumericData struct +// - UUID → replaced with bytes_le +// This is safe because execute() already copies the caller's param list +// (via list(actual_params)) before reaching this function. +// --------------------------------------------------------------------------- +std::vector DetectParamTypes(py::list& params) { + PythonObjectCache::initialize(); + + const Py_ssize_t n = py::len(params); + std::vector infos(n); + + PyObject* decimal_type = PythonObjectCache::get_decimal_class().ptr(); + PyObject* uuid_type = PythonObjectCache::get_uuid_class().ptr(); + PyObject* datetime_type = PythonObjectCache::get_datetime_class().ptr(); + PyObject* date_type = PythonObjectCache::get_date_class().ptr(); + PyObject* time_type = PythonObjectCache::get_time_class().ptr(); + + for (Py_ssize_t i = 0; i < n; ++i) { + ParamInfo& info = infos[i]; + info.inputOutputType = SQL_PARAM_INPUT; + info.isDAE = false; + + PyObject* obj = PyList_GET_ITEM(params.ptr(), i); + + // --- None --- + if (obj == Py_None) { + info.paramSQLType = SQL_UNKNOWN_TYPE; + info.paramCType = SQL_C_DEFAULT; + info.columnSize = 1; + info.decimalDigits = 0; + continue; + } + + // --- bool (must check before int, since bool is subclass of int) --- + if (PyBool_Check(obj)) { + info.paramSQLType = SQL_BIT; + info.paramCType = SQL_C_BIT; + info.columnSize = 1; + info.decimalDigits = 0; + continue; + } + + // --- int --- + if (PyLong_CheckExact(obj)) { + int overflow = 0; + int64_t val = PyLong_AsLongLongAndOverflow(obj, &overflow); + if (overflow == 0 && !PyErr_Occurred()) { + if (val >= 0 && val <= 255) { + info.paramSQLType = SQL_TINYINT; + info.paramCType = SQL_C_TINYINT; + info.columnSize = 3; + } else if (val >= -32768 && val <= 32767) { + info.paramSQLType = SQL_SMALLINT; + info.paramCType = SQL_C_SHORT; + info.columnSize = 5; + } else if (val >= -2147483648LL && val <= 2147483647LL) { + info.paramSQLType = SQL_INTEGER; + info.paramCType = SQL_C_LONG; + info.columnSize = 10; + } else { + info.paramSQLType = SQL_BIGINT; + info.paramCType = SQL_C_SBIGINT; + info.columnSize = 19; + } + } else { + PyErr_Clear(); + info.paramSQLType = SQL_BIGINT; + info.paramCType = SQL_C_SBIGINT; + info.columnSize = 19; + } + info.decimalDigits = 0; + continue; + } + + // --- float --- + if (PyFloat_CheckExact(obj)) { + info.paramSQLType = SQL_DOUBLE; + info.paramCType = SQL_C_DOUBLE; + info.columnSize = 15; + info.decimalDigits = 0; + continue; + } + + // --- str --- + if (PyUnicode_CheckExact(obj)) { + Py_ssize_t length = PyUnicode_GET_LENGTH(obj); + unsigned int kind = PyUnicode_KIND(obj); + + Py_ssize_t utf16_len; + if (kind <= PyUnicode_2BYTE_KIND) { + utf16_len = length; + } else { + utf16_len = 0; + const Py_UCS4* data = PyUnicode_4BYTE_DATA(obj); + for (Py_ssize_t j = 0; j < length; ++j) { + utf16_len += (data[j] > 0xFFFF) ? 2 : 1; + } + } + + bool is_unicode = (kind > PyUnicode_1BYTE_KIND) || + (PyUnicode_IS_COMPACT_ASCII(obj) == 0 && kind == PyUnicode_1BYTE_KIND + && PyUnicode_MAX_CHAR_VALUE(obj) > 127); + + if (utf16_len > MAX_INLINE_CHAR) { + info.isDAE = true; + info.columnSize = 0; + info.dataPtr = py::reinterpret_borrow(py::handle(obj)); + } else { + info.columnSize = is_unicode ? utf16_len : length; + info.utf16Len = utf16_len; + } + info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; + info.paramCType = is_unicode ? SQL_C_WCHAR : PARAM_C_TYPE_TEXT; + info.decimalDigits = 0; + + // Check geometry prefixes + if (length >= 5 && kind == PyUnicode_1BYTE_KIND) { + const char* ascii = (const char*)PyUnicode_1BYTE_DATA(obj); + if (strncmp(ascii, "POINT", 5) == 0 || + (length >= 10 && strncmp(ascii, "LINESTRING", 10) == 0) || + (length >= 7 && strncmp(ascii, "POLYGON", 7) == 0)) { + info.paramSQLType = SQL_WVARCHAR; + info.paramCType = SQL_C_WCHAR; + info.columnSize = length; + } + } + continue; + } + + // --- bytes / bytearray --- + if (PyBytes_CheckExact(obj) || PyByteArray_CheckExact(obj)) { + Py_ssize_t length = PyBytes_CheckExact(obj) ? PyBytes_GET_SIZE(obj) + : PyByteArray_GET_SIZE(obj); + info.paramSQLType = SQL_VARBINARY; + info.paramCType = SQL_C_BINARY; + info.decimalDigits = 0; + if (length > MAX_INLINE_BINARY) { + info.isDAE = true; + info.columnSize = 0; + info.dataPtr = py::reinterpret_borrow(py::handle(obj)); + } else { + info.columnSize = std::max(length, 1); + } + continue; + } + + // --- datetime (must check before date, since datetime is subclass of date) --- + if (PyObject_IsInstance(obj, datetime_type)) { + py::handle h(obj); + py::object tzinfo = h.attr("tzinfo"); + if (!tzinfo.is_none()) { + info.paramSQLType = SQL_SS_TIMESTAMPOFFSET; + info.paramCType = SQL_C_SS_TIMESTAMPOFFSET; + info.columnSize = 34; + info.decimalDigits = 7; + } else { + info.paramSQLType = SQL_TYPE_TIMESTAMP; + info.paramCType = SQL_C_TYPE_TIMESTAMP; + info.columnSize = 26; + info.decimalDigits = 6; + } + continue; + } + + // --- date --- + if (PyObject_IsInstance(obj, date_type)) { + info.paramSQLType = SQL_TYPE_DATE; + info.paramCType = SQL_C_TYPE_DATE; + info.columnSize = 10; + info.decimalDigits = 0; + continue; + } + + // --- time (normalized to string for binding) --- + if (PyObject_IsInstance(obj, time_type)) { + info.paramSQLType = SQL_TYPE_TIME; + info.paramCType = PARAM_C_TYPE_TEXT; + info.columnSize = 16; + info.decimalDigits = 6; + py::handle h(obj); + int hour = h.attr("hour").cast(); + int minute = h.attr("minute").cast(); + int second = h.attr("second").cast(); + int microsecond = h.attr("microsecond").cast(); + char buf[32]; + if (microsecond > 0) { + snprintf(buf, sizeof(buf), "%02d:%02d:%02d.%06d", hour, minute, second, microsecond); + } else { + snprintf(buf, sizeof(buf), "%02d:%02d:%02d", hour, minute, second); + } + py::str time_str(buf); + Py_ssize_t time_len = py::len(time_str); + info.columnSize = std::max(info.columnSize, time_len); + info.utf16Len = time_len; + PyList_SET_ITEM(params.ptr(), i, time_str.release().ptr()); + continue; + } + + // --- Decimal --- + if (PyObject_IsInstance(obj, decimal_type)) { + py::handle h(obj); + py::object as_tuple = h.attr("as_tuple")(); + py::object exponent_obj = as_tuple.attr("exponent"); + + if (py::isinstance(exponent_obj)) { + info.paramSQLType = SQL_NUMERIC; + info.paramCType = SQL_C_NUMERIC; + info.columnSize = MAX_NUMERIC_PRECISION; + info.decimalDigits = 0; + py::object numeric_data = build_numeric_data(py::reinterpret_borrow(h)); + PyList_SET_ITEM(params.ptr(), i, numeric_data.release().ptr()); + continue; + } + + py::tuple digits = as_tuple.attr("digits").cast(); + int num_digits = static_cast(py::len(digits)); + int exponent = exponent_obj.cast(); + int precision; + if (exponent >= 0) + precision = num_digits + exponent; + else if ((-exponent) <= num_digits) + precision = num_digits; + else + precision = -exponent; + + if (precision > MAX_NUMERIC_PRECISION) { + throw py::value_error( + "Precision of the numeric value is too high. " + "The maximum precision supported by SQL Server is " + + std::to_string(MAX_NUMERIC_PRECISION) + ", but got " + + std::to_string(precision) + "."); + } + + // SMALLMONEY/MONEY range — bind as formatted VARCHAR string + // to match SQL Server's fixed-point money semantics. + double dval = h.attr("__float__")().cast(); + if (dval >= SMALLMONEY_MIN && dval <= MONEY_MAX) { + py::str formatted = h.attr("__format__")(py::str("f")); + info.paramSQLType = SQL_VARCHAR; + info.paramCType = PARAM_C_TYPE_TEXT; + Py_ssize_t fmtLen = py::len(formatted); + info.columnSize = fmtLen; + info.utf16Len = fmtLen; + info.decimalDigits = 0; + PyList_SET_ITEM(params.ptr(), i, formatted.release().ptr()); + continue; + } + + // Generic numeric binding via SQL_NUMERIC_STRUCT + info.paramSQLType = SQL_NUMERIC; + info.paramCType = SQL_C_NUMERIC; + py::object numeric_data = build_numeric_data(py::reinterpret_borrow(h)); + NumericData nd = numeric_data.cast(); + info.columnSize = nd.precision; + info.decimalDigits = nd.scale; + PyList_SET_ITEM(params.ptr(), i, numeric_data.release().ptr()); + continue; + } + + // --- UUID --- + if (PyObject_IsInstance(obj, uuid_type)) { + py::handle h(obj); + py::bytes bytes_le = h.attr("bytes_le"); + info.paramSQLType = SQL_GUID; + info.paramCType = SQL_C_GUID; + info.columnSize = 16; + info.decimalDigits = 0; + PyList_SET_ITEM(params.ptr(), i, bytes_le.release().ptr()); + continue; + } + + // --- Fallback: convert to string (matches Python _map_sql_type default) --- + py::str str_val = py::str(obj); + Py_ssize_t length = py::len(str_val); + info.paramSQLType = SQL_WVARCHAR; + info.paramCType = SQL_C_WCHAR; + info.columnSize = length; + info.utf16Len = length; + info.decimalDigits = 0; + // Replace param in-place (safe: execute() copies the caller's list) + PyList_SET_ITEM(params.ptr(), i, str_val.release().ptr()); + } + + return infos; +} + +// Helper: build SQL_NUMERIC_STRUCT from Python Decimal +static py::object build_numeric_data(const py::object& decimal_param) { + py::object as_tuple = decimal_param.attr("as_tuple")(); + py::tuple digits = as_tuple.attr("digits").cast(); + int sign_val = as_tuple.attr("sign").cast(); + py::object exponent_obj = as_tuple.attr("exponent"); + + int exponent = 0; + if (py::isinstance(exponent_obj)) { + exponent = exponent_obj.cast(); + } + + int num_digits = static_cast(py::len(digits)); + int precision, scale; + if (exponent >= 0) { + precision = num_digits + exponent; + scale = 0; + } else { + scale = -exponent; + precision = std::max(num_digits, scale); + } + precision = std::max(1, std::min(precision, MAX_NUMERIC_PRECISION)); + scale = std::min(scale, precision); + + py::object py_zero = py::int_(0); + py::object int_val = py_zero; + for (auto d : digits) { + int_val = int_val * py::int_(10) + d.cast(); + } + if (exponent > 0) { + py::object multiplier = py::int_(1); + for (int j = 0; j < exponent; ++j) + multiplier = multiplier * py::int_(10); + int_val = int_val * multiplier; + } + + py::object abs_val = int_val.attr("__abs__")(); + py::bytes val_bytes = abs_val.attr("to_bytes")(py::int_(16), py::str("little")); + std::string val_str = val_bytes.cast(); + + NumericData nd; + nd.precision = static_cast(precision); + nd.scale = static_cast(scale); + nd.sign = (sign_val == 0) ? 1 : 0; + std::memset(&nd.val[0], 0, SQL_MAX_NUMERIC_LEN); + std::memcpy(&nd.val[0], val_str.data(), std::min(val_str.size(), (size_t)SQL_MAX_NUMERIC_LEN)); + + return py::cast(nd); +} + // Given a list of parameters and their ParamInfo, calls SQLBindParameter on // each of them with appropriate arguments SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params, @@ -2042,6 +2423,67 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, } } +// --------------------------------------------------------------------------- +// SQLExecuteFast — single C++ pipeline: DetectParamTypes → BindParameters → SQLExecute +// No ParamInfo objects cross the pybind11 boundary. +// +// Always uses SQLPrepare (not ExecDirect) because parameterized queries +// benefit from prepared plan reuse, and the fast path is only invoked +// when parameters are present. The use_prepare flag from the caller is +// acknowledged but overridden — this is a perf-only code path. +// --------------------------------------------------------------------------- +SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, + const std::wstring& query, + py::list params, + py::list is_stmt_prepared, + bool use_prepare, + const py::dict& encoding_settings) { + if (!statementHandle || !statementHandle->get()) { + return SQL_INVALID_HANDLE; + } + + SQLHANDLE hStmt = statementHandle->get(); + std::string charEncoding = "utf-8"; + std::string wcharEncoding = "utf-16le"; + if (encoding_settings.contains("charEncoding")) { + charEncoding = encoding_settings["charEncoding"].cast(); + } + if (encoding_settings.contains("wcharEncoding")) { + wcharEncoding = encoding_settings["wcharEncoding"].cast(); + } + + RETCODE rc; + bool already_prepared = is_stmt_prepared[0].cast(); + + // Prepare if needed (fast path always uses prepare for parameterized queries) + if (!already_prepared) { +#if defined(__APPLE__) || defined(__linux__) + std::vector queryBuffer = WStringToSQLWCHAR(query); + SQLWCHAR* queryPtr = queryBuffer.data(); +#else + SQLWCHAR* queryPtr = const_cast(query.c_str()); +#endif + { + py::gil_scoped_release release; + rc = SQLPrepare_ptr(hStmt, queryPtr, SQL_NTS); + } + if (!SQL_SUCCEEDED(rc)) return rc; + is_stmt_prepared[0] = py::bool_(true); + } + + // DetectParamTypes + BindParameters in one shot — ParamInfo stays in C++ + std::vector paramInfos = DetectParamTypes(params); + std::vector> paramBuffers; + rc = BindParameters(hStmt, params, paramInfos, paramBuffers, charEncoding); + if (!SQL_SUCCEEDED(rc)) return rc; + + { + py::gil_scoped_release release; + rc = SQLExecute_ptr(hStmt); + } + return rc; +} + SQLRETURN BindParameterArray(SQLHANDLE hStmt, const py::list& columnwise_params, const std::vector& paramInfos, size_t paramSetSize, std::vector>& paramBuffers, @@ -5803,6 +6245,10 @@ PYBIND11_MODULE(ddbc_bindings, m) { m.def("DDBCSQLExecute", &SQLExecute_wrap, "Prepare and execute T-SQL statements", py::arg("statementHandle"), py::arg("query"), py::arg("params"), py::arg("paramInfos"), py::arg("isStmtPrepared"), py::arg("usePrepare"), py::arg("encodingSettings")); + m.def("DDBCSQLExecuteFast", &SQLExecuteFast_wrap, + "Fast path: DetectParamTypes + BindParameters + SQLExecute all in C++", + py::arg("statementHandle"), py::arg("query"), py::arg("params"), + py::arg("isStmtPrepared"), py::arg("usePrepare"), py::arg("encodingSettings")); m.def("SQLExecuteMany", &SQLExecuteMany_wrap, "Execute statement with multiple parameter sets", py::arg("statementHandle"), py::arg("query"), py::arg("columnwise_params"), py::arg("paramInfos"), py::arg("paramSetSize"), py::arg("encodingSettings")); From 14ccf08f034bc3d5ecc69d5bf80e01954ad22ee5 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 29 Apr 2026 18:13:16 +0530 Subject: [PATCH 02/23] Fix DAE handling, MONEY range, and TypeError for unknown types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add complete DAE (Data-At-Execution) loop to SQLExecuteFast_wrap: SQL_NEED_DATA → SQLParamData/SQLPutData for large str/bytes/binary, matching the existing SQLExecute_wrap logic exactly - Fix DAE type assignment: non-unicode DAE strings use SQL_C_CHAR (not PARAM_C_TYPE_TEXT which maps to SQL_C_WCHAR on macOS/Linux) - Fix MONEY range lower bound: use MONEY_MIN not SMALLMONEY_MIN so negative decimals in MONEY range bind as VARCHAR (matches Python path) - Raise TypeError for unknown param types instead of silent str conversion - Add SQLFreeStmt(SQL_RESET_PARAMS) to unbind after execute --- mssql_python/pybind/ddbc_bindings.cpp | 111 +++++++++++++++++++++++--- 1 file changed, 98 insertions(+), 13 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 0fc59701..fd4ab530 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -588,15 +588,20 @@ std::vector DetectParamTypes(py::list& params) { && PyUnicode_MAX_CHAR_VALUE(obj) > 127); if (utf16_len > MAX_INLINE_CHAR) { + // DAE path: match slow-path types exactly. + // Non-unicode → SQL_VARCHAR + SQL_C_CHAR (encoded via Python codec in DAE loop) + // Unicode → SQL_WVARCHAR + SQL_C_WCHAR (wide-char streaming in DAE loop) info.isDAE = true; info.columnSize = 0; info.dataPtr = py::reinterpret_borrow(py::handle(obj)); + info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; + info.paramCType = is_unicode ? SQL_C_WCHAR : SQL_C_CHAR; } else { info.columnSize = is_unicode ? utf16_len : length; info.utf16Len = utf16_len; + info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; + info.paramCType = is_unicode ? SQL_C_WCHAR : PARAM_C_TYPE_TEXT; } - info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; - info.paramCType = is_unicode ? SQL_C_WCHAR : PARAM_C_TYPE_TEXT; info.decimalDigits = 0; // Check geometry prefixes @@ -720,7 +725,7 @@ std::vector DetectParamTypes(py::list& params) { // SMALLMONEY/MONEY range — bind as formatted VARCHAR string // to match SQL Server's fixed-point money semantics. double dval = h.attr("__float__")().cast(); - if (dval >= SMALLMONEY_MIN && dval <= MONEY_MAX) { + if (dval >= MONEY_MIN && dval <= MONEY_MAX) { py::str formatted = h.attr("__format__")(py::str("f")); info.paramSQLType = SQL_VARCHAR; info.paramCType = PARAM_C_TYPE_TEXT; @@ -755,16 +760,9 @@ std::vector DetectParamTypes(py::list& params) { continue; } - // --- Fallback: convert to string (matches Python _map_sql_type default) --- - py::str str_val = py::str(obj); - Py_ssize_t length = py::len(str_val); - info.paramSQLType = SQL_WVARCHAR; - info.paramCType = SQL_C_WCHAR; - info.columnSize = length; - info.utf16Len = length; - info.decimalDigits = 0; - // Replace param in-place (safe: execute() copies the caller's list) - PyList_SET_ITEM(params.ptr(), i, str_val.release().ptr()); + // --- Unknown type: raise TypeError (matches Python _map_sql_type) --- + throw py::type_error( + "Unsupported parameter type: The driver cannot safely convert it to a SQL type."); } return infos; @@ -2481,6 +2479,93 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, py::gil_scoped_release release; rc = SQLExecute_ptr(hStmt); } + + // DAE (Data-At-Execution) loop: when BindParameters marks a param as DAE + // (large str/bytes/binary), SQLExecute returns SQL_NEED_DATA. We must + // stream the data via SQLParamData/SQLPutData before execution completes. + if (rc == SQL_NEED_DATA) { + SQLPOINTER paramToken = nullptr; + while ((rc = SQLParamData_ptr(hStmt, ¶mToken)) == SQL_NEED_DATA) { + const ParamInfo* matchedInfo = nullptr; + for (auto& info : paramInfos) { + if (reinterpret_cast(const_cast(&info)) == paramToken) { + matchedInfo = &info; + break; + } + } + if (!matchedInfo) { + ThrowStdException("SQLExecuteFast: unrecognized paramToken from SQLParamData"); + } + const py::object& pyObj = matchedInfo->dataPtr; + if (pyObj.is_none()) { + SQLPutData_ptr(hStmt, nullptr, 0); + continue; + } + + if (py::isinstance(pyObj)) { + if (matchedInfo->paramCType == SQL_C_WCHAR) { + std::wstring wstr = pyObj.cast(); + const SQLWCHAR* dataPtr = nullptr; + size_t totalChars = 0; +#if defined(__APPLE__) || defined(__linux__) + std::vector sqlwStr = WStringToSQLWCHAR(wstr); + totalChars = sqlwStr.size() - 1; + dataPtr = sqlwStr.data(); +#else + dataPtr = wstr.c_str(); + totalChars = wstr.size(); +#endif + size_t chunkChars = DAE_CHUNK_SIZE / sizeof(SQLWCHAR); + for (size_t offset = 0; offset < totalChars; offset += chunkChars) { + size_t len = std::min(chunkChars, totalChars - offset); + rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), + static_cast(len * sizeof(SQLWCHAR))); + if (!SQL_SUCCEEDED(rc)) return rc; + } + } else if (matchedInfo->paramCType == SQL_C_CHAR) { + std::string encodedStr; + try { + py::object encoded = pyObj.attr("encode")(charEncoding, "strict"); + encodedStr = encoded.cast(); + } catch (const py::error_already_set& e) { + throw; + } + const char* dataPtr = encodedStr.data(); + size_t totalBytes = encodedStr.size(); + for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { + size_t len = std::min(static_cast(DAE_CHUNK_SIZE), + totalBytes - offset); + rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), + static_cast(len)); + if (!SQL_SUCCEEDED(rc)) return rc; + } + } else { + ThrowStdException("SQLExecuteFast: unsupported C type for str in DAE"); + } + } else if (py::isinstance(pyObj) || + py::isinstance(pyObj)) { + py::bytes b = pyObj.cast(); + std::string s = b; + const char* dataPtr = s.data(); + size_t totalBytes = s.size(); + for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { + size_t len = std::min(static_cast(DAE_CHUNK_SIZE), + totalBytes - offset); + rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), + static_cast(len)); + if (!SQL_SUCCEEDED(rc)) return rc; + } + } else { + ThrowStdException("SQLExecuteFast: DAE only supported for str or bytes"); + } + } + if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc; + } + + if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc; + + // Unbind params — buffers go out of scope after this + rc = SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); return rc; } From 8ab074c984b09488fa910241ae7e9ee38afac4af Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 30 Apr 2026 11:09:25 +0530 Subject: [PATCH 03/23] Fix MSVC warnings-as-errors: unused param and catch variable - Comment out use_prepare parameter name (C4100: unreferenced parameter) - Remove unused catch variable name (C4101: unreferenced local variable) --- mssql_python/pybind/ddbc_bindings.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index fd4ab530..19c3e2f3 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -2434,7 +2434,7 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, const std::wstring& query, py::list params, py::list is_stmt_prepared, - bool use_prepare, + bool /*use_prepare*/, const py::dict& encoding_settings) { if (!statementHandle || !statementHandle->get()) { return SQL_INVALID_HANDLE; @@ -2527,7 +2527,7 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, try { py::object encoded = pyObj.attr("encode")(charEncoding, "strict"); encodedStr = encoded.cast(); - } catch (const py::error_already_set& e) { + } catch (const py::error_already_set&) { throw; } const char* dataPtr = encodedStr.data(); From 00c85ccbaf161e7f1925501bd1df5b1dcbafde23 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 30 Apr 2026 11:23:31 +0530 Subject: [PATCH 04/23] Guard memcpy with null/length check for DevSkim DS121708 Add explicit null pointer and zero-length guards before memcpy in build_numeric_data to satisfy DevSkim code scanning rule DS121708. --- mssql_python/pybind/ddbc_bindings.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 19c3e2f3..9e904f36 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -813,7 +813,10 @@ static py::object build_numeric_data(const py::object& decimal_param) { nd.scale = static_cast(scale); nd.sign = (sign_val == 0) ? 1 : 0; std::memset(&nd.val[0], 0, SQL_MAX_NUMERIC_LEN); - std::memcpy(&nd.val[0], val_str.data(), std::min(val_str.size(), (size_t)SQL_MAX_NUMERIC_LEN)); + size_t copy_len = std::min(val_str.size(), static_cast(SQL_MAX_NUMERIC_LEN)); + if (copy_len > 0 && val_str.data() != nullptr) { + std::memcpy(&nd.val[0], val_str.data(), copy_len); + } return py::cast(nd); } From bad8acf37e1d50fb22bf7245b039fca4e4e2daaa Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 7 May 2026 12:51:08 +0530 Subject: [PATCH 05/23] STYLE: Fix black formatting in cursor.py --- mssql_python/cursor.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 783d0c69..8659b065 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -1492,9 +1492,7 @@ 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 - ) + paraminfo = self._create_parameter_types_list(param, param_info, parameters, i) parameters_type.append(paraminfo) if logger.isEnabledFor(logging.DEBUG): From c5a827fbc13595ecb58f276639b21117c784825a Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 7 May 2026 14:04:18 +0530 Subject: [PATCH 06/23] Address PR review: encoding key, subclass support, GIL, exec_rc, cursor attrs, parity test Six review fixes for SQLExecuteFast_wrap and DetectParamTypes: 1. Encoding key: read 'encoding' from settings dict (was 'charEncoding' which never matched). Only honor when ctype==SQL_C_CHAR so the default utf-16le doesn't corrupt SQL_C_CHAR DAE/inline byte paths. 2. Subclass support: PyLong_Check/PyFloat_Check/PyUnicode_Check/PyBytes_Check instead of *_CheckExact. Fixes user-defined int/str/bytes/float subclasses that were silently rejected with TypeError. Switched PyBytes_GET_SIZE to PyBytes_Size for subclass-safe length. 3. GIL release in DAE loop: SQLParamData and SQLPutData now release the GIL during each ODBC call, matching slow-path concurrency for large blobs/strings. 4. Preserve exec_rc: stash the SQLExecute return code before SQLFreeStmt so SUCCESS_WITH_INFO and other non-success-non-error codes are not clobbered by the unbind call. 5. Shallow-copy params: params = py::list(params) at function entry so DetectParamTypes' in-place PyList_SET_ITEM cannot mutate the caller's list under any future code path that might pass it directly. 6. Cursor attrs: SQLSetStmtAttr(SQL_ATTR_CURSOR_TYPE/CONCURRENCY) at entry to match slow-path semantics regardless of prior hstmt state. Also adds tests/test_023_fast_path_parity.py covering int/str/bytes/float subclasses, caller-list non-mutation, and unsupported-type TypeError. --- mssql_python/pybind/ddbc_bindings.cpp | 97 +++++++++++------ tests/test_023_fast_path_parity.py | 151 ++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 32 deletions(-) create mode 100644 tests/test_023_fast_path_parity.py diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index b9909824..8d63303e 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -526,8 +526,8 @@ std::vector DetectParamTypes(py::list& params) { continue; } - // --- int --- - if (PyLong_CheckExact(obj)) { + // --- int (allow subclasses, but bool was already caught above) --- + if (PyLong_Check(obj)) { int overflow = 0; int64_t val = PyLong_AsLongLongAndOverflow(obj, &overflow); if (overflow == 0 && !PyErr_Occurred()) { @@ -558,8 +558,8 @@ std::vector DetectParamTypes(py::list& params) { continue; } - // --- float --- - if (PyFloat_CheckExact(obj)) { + // --- float (allow subclasses) --- + if (PyFloat_Check(obj)) { info.paramSQLType = SQL_DOUBLE; info.paramCType = SQL_C_DOUBLE; info.columnSize = 15; @@ -567,8 +567,8 @@ std::vector DetectParamTypes(py::list& params) { continue; } - // --- str --- - if (PyUnicode_CheckExact(obj)) { + // --- str (allow subclasses) --- + if (PyUnicode_Check(obj)) { Py_ssize_t length = PyUnicode_GET_LENGTH(obj); unsigned int kind = PyUnicode_KIND(obj); @@ -618,10 +618,10 @@ std::vector DetectParamTypes(py::list& params) { continue; } - // --- bytes / bytearray --- - if (PyBytes_CheckExact(obj) || PyByteArray_CheckExact(obj)) { - Py_ssize_t length = PyBytes_CheckExact(obj) ? PyBytes_GET_SIZE(obj) - : PyByteArray_GET_SIZE(obj); + // --- bytes / bytearray (allow subclasses) --- + if (PyBytes_Check(obj) || PyByteArray_Check(obj)) { + Py_ssize_t length = + PyBytes_Check(obj) ? PyBytes_Size(obj) : PyByteArray_Size(obj); info.paramSQLType = SQL_VARBINARY; info.paramCType = SQL_C_BINARY; info.decimalDigits = 0; @@ -2496,15 +2496,33 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, } SQLHANDLE hStmt = statementHandle->get(); - std::string charEncoding = "utf-8"; - std::string wcharEncoding = "utf-16le"; - if (encoding_settings.contains("charEncoding")) { - charEncoding = encoding_settings["charEncoding"].cast(); + + // Configure forward-only / read-only cursor (matches slow path semantics). + if (SQLSetStmtAttr_ptr) { + SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CURSOR_TYPE, + (SQLPOINTER)SQL_CURSOR_FORWARD_ONLY, 0); + SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CONCURRENCY, + (SQLPOINTER)SQL_CONCUR_READ_ONLY, 0); } - if (encoding_settings.contains("wcharEncoding")) { - wcharEncoding = encoding_settings["wcharEncoding"].cast(); + + // Match the slow path's encoding-dict contract: keys are "encoding" and "ctype". + // Only honor the user's encoding when their preferred ctype is SQL_C_CHAR; + // otherwise the default ctype is SQL_C_WCHAR and the "encoding" value is + // meant for wide-char paths (e.g. "utf-16le") and would corrupt the + // SQL_C_CHAR DAE/inline path that operates on byte data. + std::string charEncoding = "utf-8"; + if (encoding_settings.contains("ctype") && encoding_settings.contains("encoding")) { + int ctype = encoding_settings["ctype"].cast(); + if (ctype == SQL_C_CHAR) { + charEncoding = encoding_settings["encoding"].cast(); + } } + // Shallow-copy the parameter list so DetectParamTypes' in-place + // PyList_SET_ITEM never mutates the caller's list. The cost is one + // PyList_New + N refcount bumps; cheap relative to ODBC binding. + params = py::list(params); + RETCODE rc; bool already_prepared = is_stmt_prepared[0].cast(); @@ -2538,9 +2556,16 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, // DAE (Data-At-Execution) loop: when BindParameters marks a param as DAE // (large str/bytes/binary), SQLExecute returns SQL_NEED_DATA. We must // stream the data via SQLParamData/SQLPutData before execution completes. + // GIL is released around each ODBC call to match slow-path concurrency. if (rc == SQL_NEED_DATA) { SQLPOINTER paramToken = nullptr; - while ((rc = SQLParamData_ptr(hStmt, ¶mToken)) == SQL_NEED_DATA) { + while (true) { + { + py::gil_scoped_release release; + rc = SQLParamData_ptr(hStmt, ¶mToken); + } + if (rc != SQL_NEED_DATA) break; + const ParamInfo* matchedInfo = nullptr; for (auto& info : paramInfos) { if (reinterpret_cast(const_cast(&info)) == paramToken) { @@ -2553,6 +2578,7 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, } const py::object& pyObj = matchedInfo->dataPtr; if (pyObj.is_none()) { + py::gil_scoped_release release; SQLPutData_ptr(hStmt, nullptr, 0); continue; } @@ -2573,25 +2599,27 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, size_t chunkChars = DAE_CHUNK_SIZE / sizeof(SQLWCHAR); for (size_t offset = 0; offset < totalChars; offset += chunkChars) { size_t len = std::min(chunkChars, totalChars - offset); - rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), - static_cast(len * sizeof(SQLWCHAR))); + { + py::gil_scoped_release release; + rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), + static_cast(len * sizeof(SQLWCHAR))); + } if (!SQL_SUCCEEDED(rc)) return rc; } } else if (matchedInfo->paramCType == SQL_C_CHAR) { std::string encodedStr; - try { - py::object encoded = pyObj.attr("encode")(charEncoding, "strict"); - encodedStr = encoded.cast(); - } catch (const py::error_already_set&) { - throw; - } + py::object encoded = pyObj.attr("encode")(charEncoding, "strict"); + encodedStr = encoded.cast(); const char* dataPtr = encodedStr.data(); size_t totalBytes = encodedStr.size(); for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { size_t len = std::min(static_cast(DAE_CHUNK_SIZE), totalBytes - offset); - rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), - static_cast(len)); + { + py::gil_scoped_release release; + rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), + static_cast(len)); + } if (!SQL_SUCCEEDED(rc)) return rc; } } else { @@ -2606,8 +2634,11 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { size_t len = std::min(static_cast(DAE_CHUNK_SIZE), totalBytes - offset); - rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), - static_cast(len)); + { + py::gil_scoped_release release; + rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), + static_cast(len)); + } if (!SQL_SUCCEEDED(rc)) return rc; } } else { @@ -2619,9 +2650,11 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc; - // Unbind params — buffers go out of scope after this - rc = SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); - return rc; + // Preserve the execute return code (e.g. SQL_SUCCESS_WITH_INFO) — don't + // let the SQLFreeStmt return value clobber what the caller needs to see. + SQLRETURN exec_rc = rc; + SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); + return exec_rc; } SQLRETURN BindParameterArray(SQLHANDLE hStmt, const py::list& columnwise_params, diff --git a/tests/test_023_fast_path_parity.py b/tests/test_023_fast_path_parity.py new file mode 100644 index 00000000..09da6e00 --- /dev/null +++ b/tests/test_023_fast_path_parity.py @@ -0,0 +1,151 @@ +""" +Parity tests: assert that fast path (DetectParamTypes in C++) and slow path +(_map_sql_type in Python) produce identical query results for representative +parameter types. + +The fast path runs by default. The slow path is forced by calling setinputsizes() +with a non-None entry, which triggers cursor.execute()'s slow-path branch. +""" + +import pytest +import datetime +import decimal +import uuid +from mssql_python import connect + +import os + +CONN_STR = os.environ.get( + "DB_CONNECTION_STRING", + "Server=localhost;Database=master;Uid=sa;Pwd=Str0ng@Passw0rd123;TrustServerCertificate=yes", +) + + +@pytest.fixture +def conn(): + c = connect(CONN_STR) + yield c + c.close() + + +def _roundtrip(cursor, value): + """Round-trip a single parameter through SELECT ? and return the result.""" + cursor.execute("SELECT ?", [value]) + return cursor.fetchone()[0] + + +def _force_slow_path_roundtrip(cursor, value): + """Force slow path via setinputsizes(None for that param) — any non-empty + inputsizes list with a non-None entry triggers the legacy code path.""" + # Empty list with at least one entry that's not None forces slow path. + # Using SQL_VARCHAR(8000) as an opaque "no override" placeholder. + from mssql_python import ddbc_bindings + + # A None entry means "infer", which is fine — the slow path still runs because + # _inputsizes is set (any non-empty list with at least one non-None entry). + # We need at least one non-None entry to flip use_fast_path to False. + cursor.setinputsizes([None]) # Has at least one entry but no override + # Wait — None doesn't trigger slow path. We need a real override. + # Use SQL_VARCHAR which is identity-ish for strings. + cursor.setinputsizes([(1, 0, 0)]) # (sqlType, size, decimal) tuple + cursor.execute("SELECT ?", [value]) + cursor.setinputsizes(None) # Reset + return cursor.fetchone()[0] + + +@pytest.mark.parametrize( + "value", + [ + # int range detection + 0, + 1, + 255, + 256, + 32767, + 32768, + 2147483647, + 2147483648, + -1, + -32768, + -2147483648, + # bool + True, + False, + # float + 0.0, + 3.14, + -1.5e10, + # str (ASCII inline + DAE + unicode) + "", + "hello", + "a" * 100, + # bytes + b"", + b"\x00\x01\x02", + b"x" * 100, + ], +) +def test_fast_path_roundtrip(conn, value): + """Fast path produces identical results regardless of value type.""" + cur = conn.cursor() + result = _roundtrip(cur, value) + assert ( + result == value + ), f"Roundtrip mismatch for {type(value).__name__} {value!r}: got {result!r}" + + +def test_int_subclass(conn): + """int subclasses must work (regression test for *_CheckExact bug).""" + + class MyInt(int): + pass + + cur = conn.cursor() + assert _roundtrip(cur, MyInt(42)) == 42 + + +def test_str_subclass(conn): + """str subclasses must work.""" + + class MyStr(str): + pass + + cur = conn.cursor() + assert _roundtrip(cur, MyStr("hello")) == "hello" + + +def test_bytes_subclass(conn): + """bytes subclasses must work.""" + + class MyBytes(bytes): + pass + + cur = conn.cursor() + assert _roundtrip(cur, MyBytes(b"hello")) == b"hello" + + +def test_float_subclass(conn): + """float subclasses must work.""" + + class MyFloat(float): + pass + + cur = conn.cursor() + assert _roundtrip(cur, MyFloat(3.14)) == 3.14 + + +def test_caller_param_list_not_mutated(conn): + """DetectParamTypes must not mutate the caller's parameter list.""" + cur = conn.cursor() + params = ["hello", 42, 3.14, datetime.date(2024, 1, 1), uuid.uuid4()] + snapshot = list(params) + cur.execute("SELECT ?, ?, ?, ?, ?", params) + cur.fetchone() + assert params == snapshot, f"Caller list was mutated: {params} != {snapshot}" + + +def test_unsupported_type_raises_typeerror(conn): + """Unknown parameter types must raise TypeError, matching slow path.""" + cur = conn.cursor() + with pytest.raises(TypeError): + cur.execute("SELECT ?", [{1, 2, 3}]) # set is not supported From a38ce4e94aaffe560b8c8efb58d65ad273d9524f Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 7 May 2026 14:46:24 +0530 Subject: [PATCH 07/23] Address second PR review: refcount leak, geometry+DAE, NaN, parity test Eight follow-up fixes after review feedback on c5a827f. 1. Refcount leak (BLOCKER): replace PyList_SET_ITEM (uppercase, no decref of old slot) with PyList_SetItem (decrefs old slot before stealing the new reference) in DetectParamTypes time/Decimal/UUID branches. The previous shallow-copy defense via py::list(params) was a no-op because pybind11s list constructor only inc_refs an already-list argument. 2. Geometry + DAE conflict: gate the geometry-prefix override on the not-DAE branch so a long POLYGON/POINT/LINESTRING string does not end up with isDAE=true, dataPtr set, AND a non-zero columnSize. 3. Decimal NaN/Infinity: throw ValueError instead of silently binding 0 via build_numeric_data on an empty digits tuple. 4. Time format: always emit microseconds (HH:MM:SS.ffffff), matching slow path isoformat(timespec=microseconds). 5. PyObject_IsInstance: explicit equality check so a custom __instancecheck__ that raises (returns -1) does not fall through with a Python error set. 6. Dead code: removed unused SMALLMONEY_MIN/SMALLMONEY_MAX constants and the unused utf16Len assignments in DetectParamTypes. 7. Encoding-key contract: only honor encoding_settings encoding when the user explicitly opted in via setencoding(..., ctype=SQL_C_CHAR=1). The Python layer SQL_C_CHAR constant is numerically -8 (real ODBC SQL_C_WCHAR), so by default the wide-char path is taken and encoding is irrelevant. 8. Parity test rewrite: drop the dead _force_slow_path_roundtrip helper, use the project cursor fixture instead of a hard-coded conn string, and add (a) a real fast-vs-slow parity check via setinputsizes-forced slow path, (b) a refcount-leak regression test using a Decimal subclass + weakref, (c) explicit NaN-rejection coverage. --- mssql_python/pybind/ddbc_bindings.cpp | 94 ++++++------- tests/test_023_fast_path_parity.py | 191 ++++++++++++++++---------- 2 files changed, 165 insertions(+), 120 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 8d63303e..ae0171ac 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -457,11 +457,11 @@ static constexpr int MAX_NUMERIC_PRECISION = 38; static constexpr double MONEY_MIN = -922337203685477.5808; static constexpr double MONEY_MAX = 922337203685477.5807; -// SMALLMONEY range: -214,748.3648 to 214,748.3647 -static constexpr double SMALLMONEY_MIN = -214748.3648; -static constexpr double SMALLMONEY_MAX = 214748.3647; - -// Platform-specific text C type: unixODBC requires all text as wide chars +// Platform-specific text C type: unixODBC requires all text as wide chars on +// Linux/macOS. On Windows the ODBC driver accepts SQL_C_CHAR for ASCII text. +// This matches the Python slow path's behavior (its SQL_C_CHAR constant is +// numerically -8, which is ODBC's SQL_C_WCHAR — a long-standing alias used +// throughout the Python layer). #if defined(__APPLE__) || defined(__linux__) static constexpr SQLSMALLINT PARAM_C_TYPE_TEXT = SQL_C_WCHAR; #else @@ -481,7 +481,8 @@ static py::object build_numeric_data(const py::object& decimal_param); // - bool before int (bool is a subclass of int in Python) // - datetime before date (datetime is a subclass of date) // -// Some types mutate the params list in-place via PyList_SET_ITEM: +// Some types mutate the params list in-place via PyList_SetItem (which +// decrefs the old slot before stealing the new ref): // - time → normalized to "HH:MM:SS.ffffff" string // - Decimal in MONEY range → formatted via __format__("f") // - Decimal (generic) → converted to NumericData struct @@ -589,23 +590,26 @@ std::vector DetectParamTypes(py::list& params) { if (utf16_len > MAX_INLINE_CHAR) { // DAE path: match slow-path types exactly. - // Non-unicode → SQL_VARCHAR + SQL_C_CHAR (encoded via Python codec in DAE loop) - // Unicode → SQL_WVARCHAR + SQL_C_WCHAR (wide-char streaming in DAE loop) + // Non-unicode (ASCII) → SQL_VARCHAR + PARAM_C_TYPE_TEXT + // On Linux/macOS PARAM_C_TYPE_TEXT == SQL_C_WCHAR, matching + // the slow path's SQL_C_CHAR (which is numerically -8 == + // SQL_C_WCHAR — a long-standing alias in the Python layer). + // Unicode → SQL_WVARCHAR + SQL_C_WCHAR (wide-char streaming) info.isDAE = true; info.columnSize = 0; info.dataPtr = py::reinterpret_borrow(py::handle(obj)); info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; - info.paramCType = is_unicode ? SQL_C_WCHAR : SQL_C_CHAR; + info.paramCType = is_unicode ? SQL_C_WCHAR : PARAM_C_TYPE_TEXT; } else { info.columnSize = is_unicode ? utf16_len : length; - info.utf16Len = utf16_len; info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; info.paramCType = is_unicode ? SQL_C_WCHAR : PARAM_C_TYPE_TEXT; } info.decimalDigits = 0; - // Check geometry prefixes - if (length >= 5 && kind == PyUnicode_1BYTE_KIND) { + // Check geometry prefixes (only for non-DAE strings; long geometry + // values stay on the DAE path with their already-set types). + if (!info.isDAE && length >= 5 && kind == PyUnicode_1BYTE_KIND) { const char* ascii = (const char*)PyUnicode_1BYTE_DATA(obj); if (strncmp(ascii, "POINT", 5) == 0 || (length >= 10 && strncmp(ascii, "LINESTRING", 10) == 0) || @@ -636,7 +640,7 @@ std::vector DetectParamTypes(py::list& params) { } // --- datetime (must check before date, since datetime is subclass of date) --- - if (PyObject_IsInstance(obj, datetime_type)) { + if (PyObject_IsInstance(obj, datetime_type) == 1) { py::handle h(obj); py::object tzinfo = h.attr("tzinfo"); if (!tzinfo.is_none()) { @@ -654,7 +658,7 @@ std::vector DetectParamTypes(py::list& params) { } // --- date --- - if (PyObject_IsInstance(obj, date_type)) { + if (PyObject_IsInstance(obj, date_type) == 1) { info.paramSQLType = SQL_TYPE_DATE; info.paramCType = SQL_C_TYPE_DATE; info.columnSize = 10; @@ -663,9 +667,9 @@ std::vector DetectParamTypes(py::list& params) { } // --- time (normalized to string for binding) --- - if (PyObject_IsInstance(obj, time_type)) { + if (PyObject_IsInstance(obj, time_type) == 1) { info.paramSQLType = SQL_TYPE_TIME; - info.paramCType = PARAM_C_TYPE_TEXT; + info.paramCType = PARAM_C_TYPE_TEXT; // matches slow path (its SQL_C_CHAR is -8 = SQL_C_WCHAR) info.columnSize = 16; info.decimalDigits = 6; py::handle h(obj); @@ -673,34 +677,28 @@ std::vector DetectParamTypes(py::list& params) { int minute = h.attr("minute").cast(); int second = h.attr("second").cast(); int microsecond = h.attr("microsecond").cast(); + // Always include microseconds (matches Python's isoformat(timespec="microseconds")). char buf[32]; - if (microsecond > 0) { - snprintf(buf, sizeof(buf), "%02d:%02d:%02d.%06d", hour, minute, second, microsecond); - } else { - snprintf(buf, sizeof(buf), "%02d:%02d:%02d", hour, minute, second); - } + snprintf(buf, sizeof(buf), "%02d:%02d:%02d.%06d", hour, minute, second, microsecond); py::str time_str(buf); Py_ssize_t time_len = py::len(time_str); info.columnSize = std::max(info.columnSize, time_len); - info.utf16Len = time_len; - PyList_SET_ITEM(params.ptr(), i, time_str.release().ptr()); + // PyList_SetItem (lowercase) decrefs the old slot before stealing the new + // reference, so this is safe even if `params` is shared with the caller. + PyList_SetItem(params.ptr(), i, time_str.release().ptr()); continue; } // --- Decimal --- - if (PyObject_IsInstance(obj, decimal_type)) { + if (PyObject_IsInstance(obj, decimal_type) == 1) { py::handle h(obj); py::object as_tuple = h.attr("as_tuple")(); py::object exponent_obj = as_tuple.attr("exponent"); + // NaN / Infinity / sNaN: refuse rather than silently writing 0. if (py::isinstance(exponent_obj)) { - info.paramSQLType = SQL_NUMERIC; - info.paramCType = SQL_C_NUMERIC; - info.columnSize = MAX_NUMERIC_PRECISION; - info.decimalDigits = 0; - py::object numeric_data = build_numeric_data(py::reinterpret_borrow(h)); - PyList_SET_ITEM(params.ptr(), i, numeric_data.release().ptr()); - continue; + throw py::value_error( + "Cannot bind non-finite Decimal (NaN/Infinity) as SQL NUMERIC"); } py::tuple digits = as_tuple.attr("digits").cast(); @@ -728,12 +726,11 @@ std::vector DetectParamTypes(py::list& params) { if (dval >= MONEY_MIN && dval <= MONEY_MAX) { py::str formatted = h.attr("__format__")(py::str("f")); info.paramSQLType = SQL_VARCHAR; - info.paramCType = PARAM_C_TYPE_TEXT; + info.paramCType = PARAM_C_TYPE_TEXT; // matches slow path Py_ssize_t fmtLen = py::len(formatted); info.columnSize = fmtLen; - info.utf16Len = fmtLen; info.decimalDigits = 0; - PyList_SET_ITEM(params.ptr(), i, formatted.release().ptr()); + PyList_SetItem(params.ptr(), i, formatted.release().ptr()); continue; } @@ -744,19 +741,19 @@ std::vector DetectParamTypes(py::list& params) { NumericData nd = numeric_data.cast(); info.columnSize = nd.precision; info.decimalDigits = nd.scale; - PyList_SET_ITEM(params.ptr(), i, numeric_data.release().ptr()); + PyList_SetItem(params.ptr(), i, numeric_data.release().ptr()); continue; } // --- UUID --- - if (PyObject_IsInstance(obj, uuid_type)) { + if (PyObject_IsInstance(obj, uuid_type) == 1) { py::handle h(obj); py::bytes bytes_le = h.attr("bytes_le"); info.paramSQLType = SQL_GUID; info.paramCType = SQL_C_GUID; info.columnSize = 16; info.decimalDigits = 0; - PyList_SET_ITEM(params.ptr(), i, bytes_le.release().ptr()); + PyList_SetItem(params.ptr(), i, bytes_le.release().ptr()); continue; } @@ -2505,23 +2502,26 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, (SQLPOINTER)SQL_CONCUR_READ_ONLY, 0); } - // Match the slow path's encoding-dict contract: keys are "encoding" and "ctype". - // Only honor the user's encoding when their preferred ctype is SQL_C_CHAR; - // otherwise the default ctype is SQL_C_WCHAR and the "encoding" value is - // meant for wide-char paths (e.g. "utf-16le") and would corrupt the - // SQL_C_CHAR DAE/inline path that operates on byte data. + // The encoding-settings dict has the form {"encoding": str, "ctype": int}. + // Note: the Python layer's SQL_C_CHAR constant is numerically -8, the same + // as ODBC's SQL_C_WCHAR. As a result, the only path that genuinely uses + // byte-level character encoding is when the user explicitly opts in via + // setencoding(..., ctype=mssql_python.SQL_CHAR) (which sends ctype=1, the + // real ODBC SQL_CHAR). We default to utf-8 and only honor the dict's + // encoding when ctype == 1 (real ODBC SQL_CHAR). Otherwise the user's + // "encoding" value is meant for the wide-char path and we leave it alone. std::string charEncoding = "utf-8"; if (encoding_settings.contains("ctype") && encoding_settings.contains("encoding")) { int ctype = encoding_settings["ctype"].cast(); - if (ctype == SQL_C_CHAR) { + if (ctype == SQL_C_CHAR /* real ODBC value: 1 */) { charEncoding = encoding_settings["encoding"].cast(); } } - // Shallow-copy the parameter list so DetectParamTypes' in-place - // PyList_SET_ITEM never mutates the caller's list. The cost is one - // PyList_New + N refcount bumps; cheap relative to ODBC binding. - params = py::list(params); + // The cursor.py caller always passes a fresh `list(actual_params)` so this + // function is free to mutate slots in place. Even so, every site below uses + // PyList_SetItem (which decrefs the old slot before stealing the new ref), + // so the function is safe regardless of who owns the list. RETCODE rc; bool already_prepared = is_stmt_prepared[0].cast(); diff --git a/tests/test_023_fast_path_parity.py b/tests/test_023_fast_path_parity.py index 09da6e00..331a4700 100644 --- a/tests/test_023_fast_path_parity.py +++ b/tests/test_023_fast_path_parity.py @@ -1,62 +1,54 @@ """ -Parity tests: assert that fast path (DetectParamTypes in C++) and slow path -(_map_sql_type in Python) produce identical query results for representative -parameter types. +Parity tests: assert that fast path (C++ DetectParamTypes + DDBCSQLExecuteFast) +and slow path (Python _map_sql_type + DDBCSQLExecute) produce identical query +results for representative parameter types. -The fast path runs by default. The slow path is forced by calling setinputsizes() -with a non-None entry, which triggers cursor.execute()'s slow-path branch. +Uses the project's `cursor` fixture from conftest.py so the tests work in any +environment that runs the rest of the suite. """ -import pytest import datetime import decimal +import gc import uuid -from mssql_python import connect - -import os +import weakref -CONN_STR = os.environ.get( - "DB_CONNECTION_STRING", - "Server=localhost;Database=master;Uid=sa;Pwd=Str0ng@Passw0rd123;TrustServerCertificate=yes", -) +import pytest +from mssql_python.constants import ConstantsDDBC as ddbc_sql_const -@pytest.fixture -def conn(): - c = connect(CONN_STR) - yield c - c.close() +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- -def _roundtrip(cursor, value): - """Round-trip a single parameter through SELECT ? and return the result.""" +def _fast_path_roundtrip(cursor, value): + """Default fast path: no setinputsizes.""" cursor.execute("SELECT ?", [value]) return cursor.fetchone()[0] -def _force_slow_path_roundtrip(cursor, value): - """Force slow path via setinputsizes(None for that param) — any non-empty - inputsizes list with a non-None entry triggers the legacy code path.""" - # Empty list with at least one entry that's not None forces slow path. - # Using SQL_VARCHAR(8000) as an opaque "no override" placeholder. - from mssql_python import ddbc_bindings - - # A None entry means "infer", which is fine — the slow path still runs because - # _inputsizes is set (any non-empty list with at least one non-None entry). - # We need at least one non-None entry to flip use_fast_path to False. - cursor.setinputsizes([None]) # Has at least one entry but no override - # Wait — None doesn't trigger slow path. We need a real override. - # Use SQL_VARCHAR which is identity-ish for strings. - cursor.setinputsizes([(1, 0, 0)]) # (sqlType, size, decimal) tuple - cursor.execute("SELECT ?", [value]) - cursor.setinputsizes(None) # Reset - return cursor.fetchone()[0] +def _slow_path_roundtrip(cursor, value, sql_type, column_size): + """Force the slow path by setting an explicit inputsizes entry. The fast + path is gated on `not (self._inputsizes and any(s is not None ...))`, so a + non-None tuple here flips us to the legacy Python type-detection path.""" + cursor.setinputsizes([(sql_type, column_size, 0)]) + try: + cursor.execute("SELECT ?", [value]) + return cursor.fetchone()[0] + finally: + cursor.setinputsizes(None) + + +# --------------------------------------------------------------------------- +# Fast-path coverage: representative type matrix +# --------------------------------------------------------------------------- @pytest.mark.parametrize( "value", [ - # int range detection + # int range detection (TINYINT / SMALLINT / INTEGER / BIGINT) 0, 1, 255, @@ -75,7 +67,7 @@ def _force_slow_path_roundtrip(cursor, value): 0.0, 3.14, -1.5e10, - # str (ASCII inline + DAE + unicode) + # str (ASCII inline) "", "hello", "a" * 100, @@ -85,67 +77,120 @@ def _force_slow_path_roundtrip(cursor, value): b"x" * 100, ], ) -def test_fast_path_roundtrip(conn, value): - """Fast path produces identical results regardless of value type.""" - cur = conn.cursor() - result = _roundtrip(cur, value) - assert ( - result == value - ), f"Roundtrip mismatch for {type(value).__name__} {value!r}: got {result!r}" +def test_fast_path_basic_types(cursor, value): + """Fast path round-trips representative scalar types correctly.""" + result = _fast_path_roundtrip(cursor, value) + assert result == value, ( + f"Fast-path roundtrip mismatch for {type(value).__name__} {value!r}: " f"got {result!r}" + ) + +# --------------------------------------------------------------------------- +# Subclass support — regression for the *_CheckExact bug from PR review +# --------------------------------------------------------------------------- -def test_int_subclass(conn): - """int subclasses must work (regression test for *_CheckExact bug).""" +def test_int_subclass(cursor): class MyInt(int): pass - cur = conn.cursor() - assert _roundtrip(cur, MyInt(42)) == 42 + assert _fast_path_roundtrip(cursor, MyInt(42)) == 42 -def test_str_subclass(conn): - """str subclasses must work.""" - +def test_str_subclass(cursor): class MyStr(str): pass - cur = conn.cursor() - assert _roundtrip(cur, MyStr("hello")) == "hello" - + assert _fast_path_roundtrip(cursor, MyStr("hello")) == "hello" -def test_bytes_subclass(conn): - """bytes subclasses must work.""" +def test_bytes_subclass(cursor): class MyBytes(bytes): pass - cur = conn.cursor() - assert _roundtrip(cur, MyBytes(b"hello")) == b"hello" + assert _fast_path_roundtrip(cursor, MyBytes(b"hello")) == b"hello" -def test_float_subclass(conn): - """float subclasses must work.""" - +def test_float_subclass(cursor): class MyFloat(float): pass - cur = conn.cursor() - assert _roundtrip(cur, MyFloat(3.14)) == 3.14 + assert _fast_path_roundtrip(cursor, MyFloat(3.14)) == 3.14 + +# --------------------------------------------------------------------------- +# Caller-list isolation and refcount safety +# --------------------------------------------------------------------------- -def test_caller_param_list_not_mutated(conn): + +def test_caller_param_list_not_mutated(cursor): """DetectParamTypes must not mutate the caller's parameter list.""" - cur = conn.cursor() params = ["hello", 42, 3.14, datetime.date(2024, 1, 1), uuid.uuid4()] snapshot = list(params) - cur.execute("SELECT ?, ?, ?, ?, ?", params) - cur.fetchone() + cursor.execute("SELECT ?, ?, ?, ?, ?", params) + cursor.fetchone() assert params == snapshot, f"Caller list was mutated: {params} != {snapshot}" -def test_unsupported_type_raises_typeerror(conn): - """Unknown parameter types must raise TypeError, matching slow path.""" - cur = conn.cursor() +def test_no_refcount_leak_on_in_place_replacement(cursor): + """Decimal/UUID/time params get replaced in-place inside DetectParamTypes + via PyList_SetItem. The replaced object must have its reference dropped — + a regression caught in PR review where PyList_SET_ITEM (uppercase, no + decref) leaked one reference per replaced item per execute.""" + + class TrackedDec(decimal.Decimal): + pass + + td = TrackedDec("123.45") + ref = weakref.ref(td) + params = [td] + del td # drop our local strong reference + + cursor.execute("SELECT ?", params) + cursor.fetchone() + del params # drop the list's strong reference + gc.collect() + + assert ref() is None, ( + "Decimal parameter was leaked: PyList_SetItem must decref the old " + "slot before stealing the new reference." + ) + + +# --------------------------------------------------------------------------- +# Error semantics +# --------------------------------------------------------------------------- + + +def test_unsupported_type_raises_typeerror(cursor): + """Fast path must raise TypeError for unknown parameter types — matching + the slow path's `_map_sql_type` final branch.""" with pytest.raises(TypeError): - cur.execute("SELECT ?", [{1, 2, 3}]) # set is not supported + cursor.execute("SELECT ?", [{1, 2, 3}]) # set is not bindable + + +def test_decimal_nan_rejected(cursor): + """Non-finite Decimals must raise rather than silently bind as 0.""" + with pytest.raises(Exception): # ValueError or DataError, not silent zero + cursor.execute("SELECT ?", [decimal.Decimal("NaN")]) + + +# --------------------------------------------------------------------------- +# Fast-vs-slow parity for representative types +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "value, sql_type, column_size", + [ + ("hello", ddbc_sql_const.SQL_VARCHAR.value, 5), + (42, ddbc_sql_const.SQL_INTEGER.value, 0), + (3.14, ddbc_sql_const.SQL_DOUBLE.value, 0), + (b"data", ddbc_sql_const.SQL_VARBINARY.value, 4), + ], +) +def test_fast_slow_path_parity(cursor, value, sql_type, column_size): + """Same input through both paths produces the same output.""" + fast = _fast_path_roundtrip(cursor, value) + slow = _slow_path_roundtrip(cursor, value, sql_type=sql_type, column_size=column_size) + assert fast == slow, f"Fast/slow path divergence for {value!r}: fast={fast!r} slow={slow!r}" From ba65f46ec9a180f82a5460c06f8d0ba0ef2f1800 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 1 Jul 2026 12:00:09 +0530 Subject: [PATCH 08/23] FIX: Address rubber-duck review findings in fast path - Honor use_prepare flag (was silently ignored, always preparing) - Move DetectParamTypes before SQLPrepare to prevent half-prepared state - Fix bytearray DAE crash (pybind11 bytes caster doesn't handle bytearray) - Replace lossy double MONEY comparison with exact Decimal arithmetic - Add SMALLMONEY range detection (was missing from fast path) - Handle PyObject_IsInstance error return (-1) with proper exception propagation - Clear describe cache on prepare (matching slow path) - Add edge case tests: large bytearray/bytes/string DAE, MONEY boundaries, Infinity rejection, embedded nulls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 130 +++++++++++++++++++------- tests/test_023_fast_path_parity.py | 92 ++++++++++++++++++ 2 files changed, 186 insertions(+), 36 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index f9ee77aa..3f8f17d2 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -486,9 +486,13 @@ static constexpr int MAX_INLINE_BINARY = 8000; // SQL Server maximum numeric precision static constexpr int MAX_NUMERIC_PRECISION = 38; -// MONEY range: -922,337,203,685,477.5808 to 922,337,203,685,477.5807 -static constexpr double MONEY_MIN = -922337203685477.5808; -static constexpr double MONEY_MAX = 922337203685477.5807; +// MONEY/SMALLMONEY ranges are compared using exact Decimal arithmetic (not +// double) to avoid boundary misclassification. These string constants are +// converted to Python Decimal objects at first use via PythonObjectCache. +static const char* SMALLMONEY_MIN_STR = "-214748.3648"; +static const char* SMALLMONEY_MAX_STR = "214748.3647"; +static const char* MONEY_MIN_STR = "-922337203685477.5808"; +static const char* MONEY_MAX_STR = "922337203685477.5807"; // Platform-specific text C type: unixODBC requires all text as wide chars on // Linux/macOS. On Windows the ODBC driver accepts SQL_C_CHAR for ASCII text. @@ -630,6 +634,7 @@ std::vector DetectParamTypes(py::list& params) { // Unicode → SQL_WVARCHAR + SQL_C_WCHAR (wide-char streaming) info.isDAE = true; info.columnSize = 0; + info.utf16Len = utf16_len; info.dataPtr = py::reinterpret_borrow(py::handle(obj)); info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; info.paramCType = is_unicode ? SQL_C_WCHAR : PARAM_C_TYPE_TEXT; @@ -673,7 +678,9 @@ std::vector DetectParamTypes(py::list& params) { } // --- datetime (must check before date, since datetime is subclass of date) --- - if (PyObject_IsInstance(obj, datetime_type) == 1) { + int is_datetime = PyObject_IsInstance(obj, datetime_type); + if (is_datetime == -1) throw py::error_already_set(); + if (is_datetime == 1) { py::handle h(obj); py::object tzinfo = h.attr("tzinfo"); if (!tzinfo.is_none()) { @@ -691,7 +698,9 @@ std::vector DetectParamTypes(py::list& params) { } // --- date --- - if (PyObject_IsInstance(obj, date_type) == 1) { + int is_date = PyObject_IsInstance(obj, date_type); + if (is_date == -1) throw py::error_already_set(); + if (is_date == 1) { info.paramSQLType = SQL_TYPE_DATE; info.paramCType = SQL_C_TYPE_DATE; info.columnSize = 10; @@ -700,7 +709,9 @@ std::vector DetectParamTypes(py::list& params) { } // --- time (normalized to string for binding) --- - if (PyObject_IsInstance(obj, time_type) == 1) { + int is_time = PyObject_IsInstance(obj, time_type); + if (is_time == -1) throw py::error_already_set(); + if (is_time == 1) { info.paramSQLType = SQL_TYPE_TIME; info.paramCType = PARAM_C_TYPE_TEXT; // matches slow path (its SQL_C_CHAR is -8 = SQL_C_WCHAR) info.columnSize = 16; @@ -723,7 +734,9 @@ std::vector DetectParamTypes(py::list& params) { } // --- Decimal --- - if (PyObject_IsInstance(obj, decimal_type) == 1) { + int is_decimal = PyObject_IsInstance(obj, decimal_type); + if (is_decimal == -1) throw py::error_already_set(); + if (is_decimal == 1) { py::handle h(obj); py::object as_tuple = h.attr("as_tuple")(); py::object exponent_obj = as_tuple.attr("exponent"); @@ -755,16 +768,36 @@ std::vector DetectParamTypes(py::list& params) { // SMALLMONEY/MONEY range — bind as formatted VARCHAR string // to match SQL Server's fixed-point money semantics. - double dval = h.attr("__float__")().cast(); - if (dval >= MONEY_MIN && dval <= MONEY_MAX) { - py::str formatted = h.attr("__format__")(py::str("f")); - info.paramSQLType = SQL_VARCHAR; - info.paramCType = PARAM_C_TYPE_TEXT; // matches slow path - Py_ssize_t fmtLen = py::len(formatted); - info.columnSize = fmtLen; - info.decimalDigits = 0; - PyList_SetItem(params.ptr(), i, formatted.release().ptr()); - continue; + // Use exact Decimal comparison (not double) to avoid boundary misclassification. + { + py::object decimal_mod = py::module_::import("decimal"); + py::object Decimal = decimal_mod.attr("Decimal"); + py::object py_param = py::reinterpret_borrow(py::handle(obj)); + py::object sm_min = Decimal(py::str(SMALLMONEY_MIN_STR)); + py::object sm_max = Decimal(py::str(SMALLMONEY_MAX_STR)); + py::object m_min = Decimal(py::str(MONEY_MIN_STR)); + py::object m_max = Decimal(py::str(MONEY_MAX_STR)); + + bool in_money_range = false; + // Check SMALLMONEY first (subset of MONEY) + if (py_param.attr("__ge__")(sm_min).cast() && + py_param.attr("__le__")(sm_max).cast()) { + in_money_range = true; + } else if (py_param.attr("__ge__")(m_min).cast() && + py_param.attr("__le__")(m_max).cast()) { + in_money_range = true; + } + + if (in_money_range) { + py::str formatted = h.attr("__format__")(py::str("f")); + info.paramSQLType = SQL_VARCHAR; + info.paramCType = PARAM_C_TYPE_TEXT; + Py_ssize_t fmtLen = py::len(formatted); + info.columnSize = fmtLen; + info.decimalDigits = 0; + PyList_SetItem(params.ptr(), i, formatted.release().ptr()); + continue; + } } // Generic numeric binding via SQL_NUMERIC_STRUCT @@ -779,7 +812,9 @@ std::vector DetectParamTypes(py::list& params) { } // --- UUID --- - if (PyObject_IsInstance(obj, uuid_type) == 1) { + int is_uuid = PyObject_IsInstance(obj, uuid_type); + if (is_uuid == -1) throw py::error_already_set(); + if (is_uuid == 1) { py::handle h(obj); py::bytes bytes_le = h.attr("bytes_le"); info.paramSQLType = SQL_GUID; @@ -2430,16 +2465,15 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, const std::u16stri // SQLExecuteFast — single C++ pipeline: DetectParamTypes → BindParameters → SQLExecute // No ParamInfo objects cross the pybind11 boundary. // -// Always uses SQLPrepare (not ExecDirect) because parameterized queries -// benefit from prepared plan reuse, and the fast path is only invoked -// when parameters are present. The use_prepare flag from the caller is -// acknowledged but overridden — this is a perf-only code path. +// Honors use_prepare: when true, uses SQLPrepare + SQLExecute (benefiting from +// plan reuse). When false but already prepared, reuses the existing plan. +// When false and not prepared, throws (matching slow path behavior). // --------------------------------------------------------------------------- SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, const std::u16string& query, py::list params, py::list is_stmt_prepared, - bool /*use_prepare*/, + bool use_prepare, const py::dict& encoding_settings) { if (!statementHandle || !statementHandle->get()) { return SQL_INVALID_HANDLE; @@ -2476,22 +2510,33 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, // PyList_SetItem (which decrefs the old slot before stealing the new ref), // so the function is safe regardless of who owns the list. + // Run DetectParamTypes BEFORE SQLPrepare so that type-detection errors + // (unsupported type, NaN Decimal, precision overflow) don't leave the + // cursor in a half-prepared state. + std::vector paramInfos = DetectParamTypes(params); + RETCODE rc; bool already_prepared = is_stmt_prepared[0].cast(); - // Prepare if needed (fast path always uses prepare for parameterized queries) + // Honor use_prepare flag (matching slow path behavior): + // - use_prepare=true: prepare now (or reuse if same SQL already prepared) + // - use_prepare=false + already prepared: reuse existing plan + // - use_prepare=false + not prepared: error (cannot execute unprepared) if (!already_prepared) { - SQLWCHAR* queryPtr = reinterpretU16stringAsSqlWChar(query); - { - py::gil_scoped_release release; - rc = SQLPrepare_ptr(hStmt, queryPtr, SQL_NTS); + if (use_prepare) { + SQLWCHAR* queryPtr = reinterpretU16stringAsSqlWChar(query); + { + py::gil_scoped_release release; + rc = SQLPrepare_ptr(hStmt, queryPtr, SQL_NTS); + } + if (!SQL_SUCCEEDED(rc)) return rc; + statementHandle->clearDescribeCache(); + is_stmt_prepared[0] = py::bool_(true); + } else { + ThrowStdException("Cannot execute unprepared statement"); } - if (!SQL_SUCCEEDED(rc)) return rc; - is_stmt_prepared[0] = py::bool_(true); } - // DetectParamTypes + BindParameters in one shot — ParamInfo stays in C++ - std::vector paramInfos = DetectParamTypes(params); std::vector> paramBuffers; rc = BindParameters(*statementHandle, hStmt, params, paramInfos, paramBuffers, charEncoding); if (!SQL_SUCCEEDED(rc)) return rc; @@ -2567,10 +2612,23 @@ SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, } } else if (py::isinstance(pyObj) || py::isinstance(pyObj)) { - py::bytes b = pyObj.cast(); - std::string s = b; - const char* dataPtr = s.data(); - size_t totalBytes = s.size(); + // Handle bytes and bytearray separately — pybind11's bytes + // caster does not safely convert bytearray. + const char* dataPtr = nullptr; + size_t totalBytes = 0; + std::string bytesStorage; // lifetime must span the loop + + if (py::isinstance(pyObj)) { + bytesStorage = pyObj.cast(); + dataPtr = bytesStorage.data(); + totalBytes = bytesStorage.size(); + } else { + // bytearray: use raw buffer access + PyObject* ba = pyObj.ptr(); + dataPtr = PyByteArray_AS_STRING(ba); + totalBytes = static_cast(PyByteArray_GET_SIZE(ba)); + } + for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { size_t len = std::min(static_cast(DAE_CHUNK_SIZE), totalBytes - offset); diff --git a/tests/test_023_fast_path_parity.py b/tests/test_023_fast_path_parity.py index 331a4700..7aaaa6a8 100644 --- a/tests/test_023_fast_path_parity.py +++ b/tests/test_023_fast_path_parity.py @@ -194,3 +194,95 @@ def test_fast_slow_path_parity(cursor, value, sql_type, column_size): fast = _fast_path_roundtrip(cursor, value) slow = _slow_path_roundtrip(cursor, value, sql_type=sql_type, column_size=column_size) assert fast == slow, f"Fast/slow path divergence for {value!r}: fast={fast!r} slow={slow!r}" + + +# --------------------------------------------------------------------------- +# Edge case tests (issues caught in rubber-duck review) +# --------------------------------------------------------------------------- + + +def test_large_bytearray_dae(cursor): + """Large bytearray (>8000 bytes) must stream via DAE without crashing. + This catches the pybind11 bytes-cast-from-bytearray bug.""" + large_ba = bytearray(b"\xAB" * 10000) + cursor.execute( + "SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [large_ba] + ) + result = cursor.fetchone()[0] + assert result == 10000 + + +def test_large_bytes_dae(cursor): + """Large bytes (>8000 bytes) must stream via DAE correctly.""" + large_b = b"\xCD" * 10000 + cursor.execute( + "SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [large_b] + ) + result = cursor.fetchone()[0] + assert result == 10000 + + +def test_large_string_dae(cursor): + """Large string (>4000 chars) must stream via DAE correctly.""" + large_str = "x" * 5000 + cursor.execute("SELECT LEN(?)", [large_str]) + result = cursor.fetchone()[0] + assert result == 5000 + + +def test_large_unicode_string_dae(cursor): + """Large unicode string (>4000 UTF-16 code units) streams via DAE.""" + large_str = "\u00e9" * 5000 # é = 1 UTF-16 code unit each + cursor.execute("SELECT LEN(?)", [large_str]) + result = cursor.fetchone()[0] + assert result == 5000 + + +@pytest.mark.parametrize( + "value", + [ + decimal.Decimal("-922337203685477.5808"), # MONEY_MIN boundary + decimal.Decimal("922337203685477.5807"), # MONEY_MAX boundary + decimal.Decimal("-214748.3648"), # SMALLMONEY_MIN + decimal.Decimal("214748.3647"), # SMALLMONEY_MAX + decimal.Decimal("0.01"), # typical money value + ], +) +def test_decimal_money_boundary(cursor, value): + """Decimal values at MONEY/SMALLMONEY boundaries must round-trip correctly.""" + cursor.execute("SELECT CAST(? AS DECIMAL(38,4))", [value]) + result = cursor.fetchone()[0] + assert result == value, f"MONEY boundary mismatch: sent {value}, got {result}" + + +def test_decimal_outside_money_uses_numeric(cursor): + """Decimal outside MONEY range must use SQL_NUMERIC binding.""" + # One unit above MONEY_MAX + value = decimal.Decimal("922337203685477.5808") + cursor.execute("SELECT CAST(? AS DECIMAL(38,4))", [value]) + result = cursor.fetchone()[0] + assert result == value + + +def test_decimal_infinity_rejected(cursor): + """Decimal('Infinity') must raise, not silently bind as 0.""" + with pytest.raises(Exception): + cursor.execute("SELECT ?", [decimal.Decimal("Infinity")]) + + +def test_binary_with_embedded_nulls(cursor): + """Binary data with embedded null bytes must not be truncated.""" + data = b"\x00\x01\x00\x02\x00" + cursor.execute( + "SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [data] + ) + result = cursor.fetchone()[0] + assert result == 5 + + +def test_string_with_embedded_nulls(cursor): + """String with embedded NUL chars must not be truncated.""" + value = "hello\x00world" + cursor.execute("SELECT LEN(?)", [value]) + result = cursor.fetchone()[0] + assert result == 11 From dc1460b2b68f021ad029ff1ddb99f50476236572 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 11:04:53 +0530 Subject: [PATCH 09/23] STYLE: Fix black formatting in test_023_fast_path_parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_023_fast_path_parity.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/tests/test_023_fast_path_parity.py b/tests/test_023_fast_path_parity.py index 7aaaa6a8..cbd564ba 100644 --- a/tests/test_023_fast_path_parity.py +++ b/tests/test_023_fast_path_parity.py @@ -204,20 +204,16 @@ def test_fast_slow_path_parity(cursor, value, sql_type, column_size): def test_large_bytearray_dae(cursor): """Large bytearray (>8000 bytes) must stream via DAE without crashing. This catches the pybind11 bytes-cast-from-bytearray bug.""" - large_ba = bytearray(b"\xAB" * 10000) - cursor.execute( - "SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [large_ba] - ) + large_ba = bytearray(b"\xab" * 10000) + cursor.execute("SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [large_ba]) result = cursor.fetchone()[0] assert result == 10000 def test_large_bytes_dae(cursor): """Large bytes (>8000 bytes) must stream via DAE correctly.""" - large_b = b"\xCD" * 10000 - cursor.execute( - "SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [large_b] - ) + large_b = b"\xcd" * 10000 + cursor.execute("SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [large_b]) result = cursor.fetchone()[0] assert result == 10000 @@ -241,11 +237,11 @@ def test_large_unicode_string_dae(cursor): @pytest.mark.parametrize( "value", [ - decimal.Decimal("-922337203685477.5808"), # MONEY_MIN boundary - decimal.Decimal("922337203685477.5807"), # MONEY_MAX boundary - decimal.Decimal("-214748.3648"), # SMALLMONEY_MIN - decimal.Decimal("214748.3647"), # SMALLMONEY_MAX - decimal.Decimal("0.01"), # typical money value + decimal.Decimal("-922337203685477.5808"), # MONEY_MIN boundary + decimal.Decimal("922337203685477.5807"), # MONEY_MAX boundary + decimal.Decimal("-214748.3648"), # SMALLMONEY_MIN + decimal.Decimal("214748.3647"), # SMALLMONEY_MAX + decimal.Decimal("0.01"), # typical money value ], ) def test_decimal_money_boundary(cursor, value): @@ -273,9 +269,7 @@ def test_decimal_infinity_rejected(cursor): def test_binary_with_embedded_nulls(cursor): """Binary data with embedded null bytes must not be truncated.""" data = b"\x00\x01\x00\x02\x00" - cursor.execute( - "SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [data] - ) + cursor.execute("SELECT DATALENGTH(CAST(? AS VARBINARY(MAX)))", [data]) result = cursor.fetchone()[0] assert result == 5 From 5cf016ec430a6a001280884f33ecffa7b482a813 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 11:15:45 +0530 Subject: [PATCH 10/23] PERF: Convert DetectParamTypes to raw CPython API - Replace pybind11 .attr()/.cast<>() with raw CPython calls throughout DetectParamTypes and build_numeric_data - datetime/date/time: use PyDateTime_Check/PyDate_Check/PyTime_Check macros and PyDateTime_TIME_GET_* accessors (requires PyDateTime_IMPORT) - Decimal: PyObject_CallMethod/GetAttrString/RichCompareBool instead of py::module_::import + py::object .attr() chains - UUID: PyObject_GetAttrString("bytes_le") instead of py::handle .attr() - Cache MONEY/SMALLMONEY Decimal bounds in PythonObjectCache (constructed once at init, not per-call) using cached Python-side constants - Replace magic int range numbers with UINT8_MAX/INT16_MIN/MAX/INT32_MIN/MAX - Proper Py_DECREF cleanup on all error paths in build_numeric_data Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 369 +++++++++++++++++++------- 1 file changed, 268 insertions(+), 101 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 3f8f17d2..8c6d03d2 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -19,6 +19,7 @@ #include // std::setw, std::setfill #include #include // std::forward +#include // CPython datetime API (PyDateTime_IMPORT, PyDateTime_GET_*, etc.) //------------------------------------------------------------------------------------------------- @@ -119,10 +120,19 @@ static py::object date_class; static py::object time_class; static py::object decimal_class; static py::object uuid_class; +static PyObject* money_min = nullptr; +static PyObject* money_max = nullptr; +static PyObject* smallmoney_min = nullptr; +static PyObject* smallmoney_max = nullptr; static bool cache_initialized = false; void initialize() { if (!cache_initialized) { + PyDateTime_IMPORT; + if (PyDateTimeAPI == nullptr) { + throw py::error_already_set(); + } + auto datetime_module = py::module_::import("datetime"); datetime_class = datetime_module.attr("datetime"); date_class = datetime_module.attr("date"); @@ -134,6 +144,23 @@ void initialize() { auto uuid_module = py::module_::import("uuid"); uuid_class = uuid_module.attr("UUID"); + PyObject* Decimal = decimal_class.ptr(); + smallmoney_min = PyObject_CallFunction(Decimal, "s", "-214748.3648"); + smallmoney_max = PyObject_CallFunction(Decimal, "s", "214748.3647"); + money_min = PyObject_CallFunction(Decimal, "s", "-922337203685477.5808"); + money_max = PyObject_CallFunction(Decimal, "s", "922337203685477.5807"); + if (!smallmoney_min || !smallmoney_max || !money_min || !money_max) { + Py_XDECREF(smallmoney_min); + Py_XDECREF(smallmoney_max); + Py_XDECREF(money_min); + Py_XDECREF(money_max); + smallmoney_min = nullptr; + smallmoney_max = nullptr; + money_min = nullptr; + money_max = nullptr; + throw py::error_already_set(); + } + cache_initialized = true; } } @@ -486,14 +513,6 @@ static constexpr int MAX_INLINE_BINARY = 8000; // SQL Server maximum numeric precision static constexpr int MAX_NUMERIC_PRECISION = 38; -// MONEY/SMALLMONEY ranges are compared using exact Decimal arithmetic (not -// double) to avoid boundary misclassification. These string constants are -// converted to Python Decimal objects at first use via PythonObjectCache. -static const char* SMALLMONEY_MIN_STR = "-214748.3648"; -static const char* SMALLMONEY_MAX_STR = "214748.3647"; -static const char* MONEY_MIN_STR = "-922337203685477.5808"; -static const char* MONEY_MAX_STR = "922337203685477.5807"; - // Platform-specific text C type: unixODBC requires all text as wide chars on // Linux/macOS. On Windows the ODBC driver accepts SQL_C_CHAR for ASCII text. // This matches the Python slow path's behavior (its SQL_C_CHAR constant is @@ -530,14 +549,11 @@ static py::object build_numeric_data(const py::object& decimal_param); std::vector DetectParamTypes(py::list& params) { PythonObjectCache::initialize(); - const Py_ssize_t n = py::len(params); + const Py_ssize_t n = PyList_GET_SIZE(params.ptr()); std::vector infos(n); PyObject* decimal_type = PythonObjectCache::get_decimal_class().ptr(); - PyObject* uuid_type = PythonObjectCache::get_uuid_class().ptr(); - PyObject* datetime_type = PythonObjectCache::get_datetime_class().ptr(); - PyObject* date_type = PythonObjectCache::get_date_class().ptr(); - PyObject* time_type = PythonObjectCache::get_time_class().ptr(); + PyObject* uuid_type = PythonObjectCache::get_uuid_class().ptr(); for (Py_ssize_t i = 0; i < n; ++i) { ParamInfo& info = infos[i]; @@ -569,15 +585,15 @@ std::vector DetectParamTypes(py::list& params) { int overflow = 0; int64_t val = PyLong_AsLongLongAndOverflow(obj, &overflow); if (overflow == 0 && !PyErr_Occurred()) { - if (val >= 0 && val <= 255) { + if (val >= 0 && val <= UINT8_MAX) { info.paramSQLType = SQL_TINYINT; info.paramCType = SQL_C_TINYINT; info.columnSize = 3; - } else if (val >= -32768 && val <= 32767) { + } else if (val >= INT16_MIN && val <= INT16_MAX) { info.paramSQLType = SQL_SMALLINT; info.paramCType = SQL_C_SHORT; info.columnSize = 5; - } else if (val >= -2147483648LL && val <= 2147483647LL) { + } else if (val >= INT32_MIN && val <= INT32_MAX) { info.paramSQLType = SQL_INTEGER; info.paramCType = SQL_C_LONG; info.columnSize = 10; @@ -621,9 +637,10 @@ std::vector DetectParamTypes(py::list& params) { } } - bool is_unicode = (kind > PyUnicode_1BYTE_KIND) || - (PyUnicode_IS_COMPACT_ASCII(obj) == 0 && kind == PyUnicode_1BYTE_KIND - && PyUnicode_MAX_CHAR_VALUE(obj) > 127); + bool is_unicode = + (kind > PyUnicode_1BYTE_KIND) || + (PyUnicode_IS_COMPACT_ASCII(obj) == 0 && kind == PyUnicode_1BYTE_KIND && + PyUnicode_MAX_CHAR_VALUE(obj) > 127); if (utf16_len > MAX_INLINE_CHAR) { // DAE path: match slow-path types exactly. @@ -662,8 +679,7 @@ std::vector DetectParamTypes(py::list& params) { // --- bytes / bytearray (allow subclasses) --- if (PyBytes_Check(obj) || PyByteArray_Check(obj)) { - Py_ssize_t length = - PyBytes_Check(obj) ? PyBytes_Size(obj) : PyByteArray_Size(obj); + Py_ssize_t length = PyBytes_Check(obj) ? PyBytes_Size(obj) : PyByteArray_Size(obj); info.paramSQLType = SQL_VARBINARY; info.paramCType = SQL_C_BINARY; info.decimalDigits = 0; @@ -678,12 +694,12 @@ std::vector DetectParamTypes(py::list& params) { } // --- datetime (must check before date, since datetime is subclass of date) --- - int is_datetime = PyObject_IsInstance(obj, datetime_type); - if (is_datetime == -1) throw py::error_already_set(); - if (is_datetime == 1) { - py::handle h(obj); - py::object tzinfo = h.attr("tzinfo"); - if (!tzinfo.is_none()) { + if (PyDateTime_Check(obj)) { + PyObject* tzinfo = PyObject_GetAttrString(obj, "tzinfo"); + if (!tzinfo) throw py::error_already_set(); + bool has_tz = (tzinfo != Py_None); + Py_DECREF(tzinfo); + if (has_tz) { info.paramSQLType = SQL_SS_TIMESTAMPOFFSET; info.paramCType = SQL_C_SS_TIMESTAMPOFFSET; info.columnSize = 34; @@ -698,9 +714,7 @@ std::vector DetectParamTypes(py::list& params) { } // --- date --- - int is_date = PyObject_IsInstance(obj, date_type); - if (is_date == -1) throw py::error_already_set(); - if (is_date == 1) { + if (PyDate_Check(obj)) { info.paramSQLType = SQL_TYPE_DATE; info.paramCType = SQL_C_TYPE_DATE; info.columnSize = 10; @@ -709,27 +723,26 @@ std::vector DetectParamTypes(py::list& params) { } // --- time (normalized to string for binding) --- - int is_time = PyObject_IsInstance(obj, time_type); - if (is_time == -1) throw py::error_already_set(); - if (is_time == 1) { + if (PyTime_Check(obj)) { info.paramSQLType = SQL_TYPE_TIME; - info.paramCType = PARAM_C_TYPE_TEXT; // matches slow path (its SQL_C_CHAR is -8 = SQL_C_WCHAR) + info.paramCType = + PARAM_C_TYPE_TEXT; // matches slow path (its SQL_C_CHAR is -8 = SQL_C_WCHAR) info.columnSize = 16; info.decimalDigits = 6; - py::handle h(obj); - int hour = h.attr("hour").cast(); - int minute = h.attr("minute").cast(); - int second = h.attr("second").cast(); - int microsecond = h.attr("microsecond").cast(); + int hour = PyDateTime_TIME_GET_HOUR(obj); + int minute = PyDateTime_TIME_GET_MINUTE(obj); + int second = PyDateTime_TIME_GET_SECOND(obj); + int microsecond = PyDateTime_TIME_GET_MICROSECOND(obj); // Always include microseconds (matches Python's isoformat(timespec="microseconds")). char buf[32]; snprintf(buf, sizeof(buf), "%02d:%02d:%02d.%06d", hour, minute, second, microsecond); - py::str time_str(buf); - Py_ssize_t time_len = py::len(time_str); + PyObject* time_str = PyUnicode_FromString(buf); + if (!time_str) throw py::error_already_set(); + Py_ssize_t time_len = PyUnicode_GET_LENGTH(time_str); info.columnSize = std::max(info.columnSize, time_len); // PyList_SetItem (lowercase) decrefs the old slot before stealing the new // reference, so this is safe even if `params` is shared with the caller. - PyList_SetItem(params.ptr(), i, time_str.release().ptr()); + PyList_SetItem(params.ptr(), i, time_str); continue; } @@ -737,28 +750,50 @@ std::vector DetectParamTypes(py::list& params) { int is_decimal = PyObject_IsInstance(obj, decimal_type); if (is_decimal == -1) throw py::error_already_set(); if (is_decimal == 1) { - py::handle h(obj); - py::object as_tuple = h.attr("as_tuple")(); - py::object exponent_obj = as_tuple.attr("exponent"); + PyObject* as_tuple = PyObject_CallMethod(obj, "as_tuple", NULL); + if (!as_tuple) throw py::error_already_set(); + + PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); + if (!exponent_obj) { + Py_DECREF(as_tuple); + throw py::error_already_set(); + } // NaN / Infinity / sNaN: refuse rather than silently writing 0. - if (py::isinstance(exponent_obj)) { + if (PyUnicode_Check(exponent_obj)) { + Py_DECREF(exponent_obj); + Py_DECREF(as_tuple); throw py::value_error( "Cannot bind non-finite Decimal (NaN/Infinity) as SQL NUMERIC"); } - py::tuple digits = as_tuple.attr("digits").cast(); - int num_digits = static_cast(py::len(digits)); - int exponent = exponent_obj.cast(); + PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); + if (!digits_obj) { + Py_DECREF(exponent_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + + Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj); + int exponent = static_cast(PyLong_AsLong(exponent_obj)); + Py_DECREF(exponent_obj); + if (exponent == -1 && PyErr_Occurred()) { + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + int precision; if (exponent >= 0) - precision = num_digits + exponent; + precision = static_cast(num_digits) + exponent; else if ((-exponent) <= num_digits) - precision = num_digits; + precision = static_cast(num_digits); else precision = -exponent; if (precision > MAX_NUMERIC_PRECISION) { + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); throw py::value_error( "Precision of the numeric value is too high. " "The maximum precision supported by SQL Server is " + @@ -769,41 +804,53 @@ std::vector DetectParamTypes(py::list& params) { // SMALLMONEY/MONEY range — bind as formatted VARCHAR string // to match SQL Server's fixed-point money semantics. // Use exact Decimal comparison (not double) to avoid boundary misclassification. - { - py::object decimal_mod = py::module_::import("decimal"); - py::object Decimal = decimal_mod.attr("Decimal"); - py::object py_param = py::reinterpret_borrow(py::handle(obj)); - py::object sm_min = Decimal(py::str(SMALLMONEY_MIN_STR)); - py::object sm_max = Decimal(py::str(SMALLMONEY_MAX_STR)); - py::object m_min = Decimal(py::str(MONEY_MIN_STR)); - py::object m_max = Decimal(py::str(MONEY_MAX_STR)); - - bool in_money_range = false; - // Check SMALLMONEY first (subset of MONEY) - if (py_param.attr("__ge__")(sm_min).cast() && - py_param.attr("__le__")(sm_max).cast()) { - in_money_range = true; - } else if (py_param.attr("__ge__")(m_min).cast() && - py_param.attr("__le__")(m_max).cast()) { + bool in_money_range = false; + int cmp_ge = PyObject_RichCompareBool(obj, PythonObjectCache::smallmoney_min, Py_GE); + int cmp_le = PyObject_RichCompareBool(obj, PythonObjectCache::smallmoney_max, Py_LE); + if (cmp_ge == -1 || cmp_le == -1) { + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + if (cmp_ge == 1 && cmp_le == 1) { + in_money_range = true; + } else { + cmp_ge = PyObject_RichCompareBool(obj, PythonObjectCache::money_min, Py_GE); + cmp_le = PyObject_RichCompareBool(obj, PythonObjectCache::money_max, Py_LE); + if (cmp_ge == -1 || cmp_le == -1) { + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + if (cmp_ge == 1 && cmp_le == 1) { in_money_range = true; } + } - if (in_money_range) { - py::str formatted = h.attr("__format__")(py::str("f")); - info.paramSQLType = SQL_VARCHAR; - info.paramCType = PARAM_C_TYPE_TEXT; - Py_ssize_t fmtLen = py::len(formatted); - info.columnSize = fmtLen; - info.decimalDigits = 0; - PyList_SetItem(params.ptr(), i, formatted.release().ptr()); - continue; + if (in_money_range) { + PyObject* formatted = PyObject_CallMethod(obj, "__format__", "s", "f"); + if (!formatted) { + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); } + info.paramSQLType = SQL_VARCHAR; + info.paramCType = PARAM_C_TYPE_TEXT; + info.columnSize = PyUnicode_GET_LENGTH(formatted); + info.decimalDigits = 0; + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + PyList_SetItem(params.ptr(), i, formatted); + continue; } // Generic numeric binding via SQL_NUMERIC_STRUCT info.paramSQLType = SQL_NUMERIC; info.paramCType = SQL_C_NUMERIC; - py::object numeric_data = build_numeric_data(py::reinterpret_borrow(h)); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + py::object numeric_data = + build_numeric_data(py::reinterpret_borrow(py::handle(obj))); NumericData nd = numeric_data.cast(); info.columnSize = nd.precision; info.decimalDigits = nd.scale; @@ -815,13 +862,13 @@ std::vector DetectParamTypes(py::list& params) { int is_uuid = PyObject_IsInstance(obj, uuid_type); if (is_uuid == -1) throw py::error_already_set(); if (is_uuid == 1) { - py::handle h(obj); - py::bytes bytes_le = h.attr("bytes_le"); + PyObject* bytes_le = PyObject_GetAttrString(obj, "bytes_le"); + if (!bytes_le) throw py::error_already_set(); info.paramSQLType = SQL_GUID; info.paramCType = SQL_C_GUID; info.columnSize = 16; info.decimalDigits = 0; - PyList_SetItem(params.ptr(), i, bytes_le.release().ptr()); + PyList_SetItem(params.ptr(), i, bytes_le); continue; } @@ -835,17 +882,50 @@ std::vector DetectParamTypes(py::list& params) { // Helper: build SQL_NUMERIC_STRUCT from Python Decimal static py::object build_numeric_data(const py::object& decimal_param) { - py::object as_tuple = decimal_param.attr("as_tuple")(); - py::tuple digits = as_tuple.attr("digits").cast(); - int sign_val = as_tuple.attr("sign").cast(); - py::object exponent_obj = as_tuple.attr("exponent"); + PyObject* as_tuple = PyObject_CallMethod(decimal_param.ptr(), "as_tuple", NULL); + if (!as_tuple) throw py::error_already_set(); + + PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); + if (!digits_obj) { + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + PyObject* sign_obj = PyObject_GetAttrString(as_tuple, "sign"); + if (!sign_obj) { + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); + if (!exponent_obj) { + Py_DECREF(sign_obj); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + + int sign_val = static_cast(PyLong_AsLong(sign_obj)); + Py_DECREF(sign_obj); + if (sign_val == -1 && PyErr_Occurred()) { + Py_DECREF(exponent_obj); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } int exponent = 0; - if (py::isinstance(exponent_obj)) { - exponent = exponent_obj.cast(); + if (PyLong_Check(exponent_obj)) { + exponent = static_cast(PyLong_AsLong(exponent_obj)); + if (exponent == -1 && PyErr_Occurred()) { + Py_DECREF(exponent_obj); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } } + Py_DECREF(exponent_obj); - int num_digits = static_cast(py::len(digits)); + int num_digits = static_cast(PyTuple_GET_SIZE(digits_obj)); int precision, scale; if (exponent >= 0) { precision = num_digits + exponent; @@ -857,31 +937,118 @@ static py::object build_numeric_data(const py::object& decimal_param) { precision = std::max(1, std::min(precision, MAX_NUMERIC_PRECISION)); scale = std::min(scale, precision); - py::object py_zero = py::int_(0); - py::object int_val = py_zero; - for (auto d : digits) { - int_val = int_val * py::int_(10) + d.cast(); + PyObject* py_ten = PyLong_FromLong(10); + PyObject* int_val = PyLong_FromLong(0); + if (!py_ten || !int_val) { + Py_XDECREF(int_val); + Py_XDECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + + const Py_ssize_t digit_count = PyTuple_GET_SIZE(digits_obj); + for (Py_ssize_t i = 0; i < digit_count; ++i) { + PyObject* digit_obj = PyTuple_GET_ITEM(digits_obj, i); + long digit = PyLong_AsLong(digit_obj); + if (digit == -1 && PyErr_Occurred()) { + Py_DECREF(int_val); + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + + PyObject* multiplied = PyNumber_Multiply(int_val, py_ten); + Py_DECREF(int_val); + if (!multiplied) { + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + + PyObject* py_digit = PyLong_FromLong(digit); + if (!py_digit) { + Py_DECREF(multiplied); + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + + int_val = PyNumber_Add(multiplied, py_digit); + Py_DECREF(multiplied); + Py_DECREF(py_digit); + if (!int_val) { + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } } + if (exponent > 0) { - py::object multiplier = py::int_(1); - for (int j = 0; j < exponent; ++j) - multiplier = multiplier * py::int_(10); - int_val = int_val * multiplier; + PyObject* multiplier = PyLong_FromLong(1); + if (!multiplier) { + Py_DECREF(int_val); + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + for (int j = 0; j < exponent; ++j) { + PyObject* next_multiplier = PyNumber_Multiply(multiplier, py_ten); + Py_DECREF(multiplier); + if (!next_multiplier) { + Py_DECREF(int_val); + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + multiplier = next_multiplier; + } + PyObject* scaled_val = PyNumber_Multiply(int_val, multiplier); + Py_DECREF(multiplier); + Py_DECREF(int_val); + if (!scaled_val) { + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::error_already_set(); + } + int_val = scaled_val; } - py::object abs_val = int_val.attr("__abs__")(); - py::bytes val_bytes = abs_val.attr("to_bytes")(py::int_(16), py::str("little")); - std::string val_str = val_bytes.cast(); + PyObject* abs_val = PyNumber_Absolute(int_val); + Py_DECREF(int_val); + Py_DECREF(py_ten); + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + if (!abs_val) throw py::error_already_set(); + + PyObject* val_bytes = PyObject_CallMethod(abs_val, "to_bytes", "is", 16, "little"); + Py_DECREF(abs_val); + if (!val_bytes) throw py::error_already_set(); + + char* val_buf = nullptr; + Py_ssize_t val_size = 0; + if (PyBytes_AsStringAndSize(val_bytes, &val_buf, &val_size) == -1) { + Py_DECREF(val_bytes); + throw py::error_already_set(); + } NumericData nd; nd.precision = static_cast(precision); nd.scale = static_cast(scale); nd.sign = (sign_val == 0) ? 1 : 0; std::memset(&nd.val[0], 0, SQL_MAX_NUMERIC_LEN); - size_t copy_len = std::min(val_str.size(), static_cast(SQL_MAX_NUMERIC_LEN)); - if (copy_len > 0 && val_str.data() != nullptr) { - std::memcpy(&nd.val[0], val_str.data(), copy_len); + size_t copy_len = std::min(static_cast(val_size), static_cast(SQL_MAX_NUMERIC_LEN)); + if (copy_len > 0 && val_buf != nullptr) { + std::memcpy(&nd.val[0], val_buf, copy_len); } + Py_DECREF(val_bytes); return py::cast(nd); } From 8e8d622b92cc194d74395ebc260465f18a6a8a07 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 11:20:04 +0530 Subject: [PATCH 11/23] FIX: Use memcpy_s on MSVC for CodeQL DS121708 compliance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 8c6d03d2..a8689b42 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1046,7 +1046,12 @@ static py::object build_numeric_data(const py::object& decimal_param) { std::memset(&nd.val[0], 0, SQL_MAX_NUMERIC_LEN); size_t copy_len = std::min(static_cast(val_size), static_cast(SQL_MAX_NUMERIC_LEN)); if (copy_len > 0 && val_buf != nullptr) { - std::memcpy(&nd.val[0], val_buf, copy_len); +#ifdef _MSC_VER + memcpy_s(&nd.val[0], SQL_MAX_NUMERIC_LEN, val_buf, copy_len); +#else + // copy_len is bounded to SQL_MAX_NUMERIC_LEN above — safe by construction + std::memcpy(&nd.val[0], val_buf, copy_len); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling) +#endif } Py_DECREF(val_bytes); From cbd2f00106d55822460a2c47f23011ffba3233d0 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 11:36:27 +0530 Subject: [PATCH 12/23] =?UTF-8?q?REFACTOR:=20Rename=20DDBCSQLExecuteFast?= =?UTF-8?q?=20=E2=86=92=20DDBCSQLExecute,=20old=20=E2=86=92=20DDBCSQLExecu?= =?UTF-8?q?teLegacy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new C++ pipeline is the primary path (99% of calls). The old function is the legacy fallback for setinputsizes users only. Naming should reflect this. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/cursor.py | 8 ++++---- mssql_python/pybind/ddbc_bindings.cpp | 11 ++++++----- tests/test_010_pybind_functions.py | 1 + tests/test_023_fast_path_parity.py | 4 ++-- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 2c648c86..b147440d 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -1506,14 +1506,14 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state self.is_stmt_prepared = [False] effective_use_prepare = use_prepare and not same_sql - # Fast path: when no inputsizes override, do type detection + bind + execute + # Primary path: when no inputsizes override, do type detection + bind + execute # entirely in C++. ParamInfo never crosses the pybind11 boundary. use_fast_path = parameters and not ( self._inputsizes and any(s is not None for s in self._inputsizes) ) if use_fast_path: - ret = ddbc_bindings.DDBCSQLExecuteFast( + ret = ddbc_bindings.DDBCSQLExecute( self.hstmt, operation, parameters, @@ -1522,7 +1522,7 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state encoding_settings, ) else: - # Slow path: Python-side type detection (used when setinputsizes overrides are present) + # Legacy path: Python-side type detection (used when setinputsizes overrides are present) parameters_type = [] if parameters: param_info = ddbc_bindings.ParamInfo @@ -1545,7 +1545,7 @@ def execute( # pylint: disable=too-many-locals,too-many-branches,too-many-state parameters_type[i].inputOutputType, ) - ret = ddbc_bindings.DDBCSQLExecute( + ret = ddbc_bindings.DDBCSQLExecuteLegacy( self.hstmt, operation, parameters, diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index a8689b42..f2d7eb5c 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -2396,7 +2396,7 @@ SQLRETURN SQLTables_wrap(SqlHandlePtr StatementHandle, const std::u16string& cat // statement and binds the parameters. Otherwise, it executes the query // directly. 'usePrepare' parameter can be used to disable the prepare step for // queries that might already be prepared in a previous call. -SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, const std::u16string& query, +SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u16string& query, const py::list& params, std::vector& paramInfos, py::list& isStmtPrepared, const bool usePrepare, const py::dict& encodingSettings) { @@ -2641,7 +2641,7 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, const std::u16stri // plan reuse). When false but already prepared, reuses the existing plan. // When false and not prepared, throws (matching slow path behavior). // --------------------------------------------------------------------------- -SQLRETURN SQLExecuteFast_wrap(const SqlHandlePtr statementHandle, +SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, const std::u16string& query, py::list params, py::list is_stmt_prepared, @@ -6697,11 +6697,12 @@ PYBIND11_MODULE(ddbc_bindings, m) { m.def("enable_pooling", &enable_pooling, "Enable global connection pooling"); m.def("close_pooling", []() { ConnectionPoolManager::getInstance().closePools(); }); m.def("DDBCSQLExecDirect", &SQLExecDirect_wrap, "Execute a SQL query directly"); - m.def("DDBCSQLExecute", &SQLExecute_wrap, "Prepare and execute T-SQL statements", + m.def("DDBCSQLExecuteLegacy", &SQLExecuteLegacy_wrap, + "Legacy path: accepts pre-built ParamInfo from Python (used with setinputsizes)", py::arg("statementHandle"), py::arg("query"), py::arg("params"), py::arg("paramInfos"), py::arg("isStmtPrepared"), py::arg("usePrepare"), py::arg("encodingSettings")); - m.def("DDBCSQLExecuteFast", &SQLExecuteFast_wrap, - "Fast path: DetectParamTypes + BindParameters + SQLExecute all in C++", + m.def("DDBCSQLExecute", &SQLExecute_wrap, + "Primary path: DetectParamTypes + BindParameters + SQLExecute all in C++", py::arg("statementHandle"), py::arg("query"), py::arg("params"), py::arg("isStmtPrepared"), py::arg("usePrepare"), py::arg("encodingSettings")); m.def("SQLExecuteMany", &SQLExecuteMany_wrap, "Execute statement with multiple parameter sets", diff --git a/tests/test_010_pybind_functions.py b/tests/test_010_pybind_functions.py index 106b64ca..b7374112 100644 --- a/tests/test_010_pybind_functions.py +++ b/tests/test_010_pybind_functions.py @@ -513,6 +513,7 @@ def test_all_exposed_functions_exist(self): "DDBCSetDecimalSeparator", "DDBCSQLExecDirect", "DDBCSQLExecute", + "DDBCSQLExecuteLegacy", "DDBCSQLRowCount", "DDBCSQLFetch", "DDBCSQLNumResultCols", diff --git a/tests/test_023_fast_path_parity.py b/tests/test_023_fast_path_parity.py index cbd564ba..c4f2b383 100644 --- a/tests/test_023_fast_path_parity.py +++ b/tests/test_023_fast_path_parity.py @@ -1,6 +1,6 @@ """ -Parity tests: assert that fast path (C++ DetectParamTypes + DDBCSQLExecuteFast) -and slow path (Python _map_sql_type + DDBCSQLExecute) produce identical query +Parity tests: assert that the primary path (C++ DetectParamTypes + DDBCSQLExecute) +and legacy path (Python _map_sql_type + DDBCSQLExecuteLegacy) produce identical query results for representative parameter types. Uses the project's `cursor` fixture from conftest.py so the tests work in any From fa0e866813d095b2bb8b387070e606a925297928 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 11:45:54 +0530 Subject: [PATCH 13/23] REFACTOR: Convert PythonObjectCache + ParamInfo::dataPtr to raw CPython MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes unnecessary pybind11 ↔ CPython round-trips in the hot path: - PythonObjectCache types stored as PyObject* (not py::object) - ParamInfo::dataPtr is raw PyObject* with explicit refcount management - DetectParamTypes takes PyObject* directly (not py::list&) - build_numeric_data returns NumericData struct (not py::object) - Added contextual comments explaining non-obvious design decisions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 375 +++++++++++++++++--------- 1 file changed, 255 insertions(+), 120 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index f2d7eb5c..4681c1dd 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -103,7 +103,8 @@ inline int EffectiveCharCtypeForFetch(int charCtype, const std::string& charEnco } namespace PythonObjectCache { -py::object get_time_class(); +PyObject* get_time_class(); +py::object get_time_class_obj(); } //------------------------------------------------------------------------------------------------- @@ -115,11 +116,13 @@ py::object get_time_class(); // embedded in macro //------------------------------------------------------------------------------------------------- namespace PythonObjectCache { -static py::object datetime_class; -static py::object date_class; -static py::object time_class; -static py::object decimal_class; -static py::object uuid_class; +// Cached as raw PyObject* to avoid pybind11 refcount wrapper overhead on every access. +// These are immortal (never DECREFed) — module-lifetime singletons, same as CPython's type objects. +static PyObject* datetime_class = nullptr; +static PyObject* date_class = nullptr; +static PyObject* time_class = nullptr; +static PyObject* decimal_class = nullptr; +static PyObject* uuid_class = nullptr; static PyObject* money_min = nullptr; static PyObject* money_max = nullptr; static PyObject* smallmoney_min = nullptr; @@ -133,18 +136,27 @@ void initialize() { throw py::error_already_set(); } - auto datetime_module = py::module_::import("datetime"); - datetime_class = datetime_module.attr("datetime"); - date_class = datetime_module.attr("date"); - time_class = datetime_module.attr("time"); - - auto decimal_module = py::module_::import("decimal"); - decimal_class = decimal_module.attr("Decimal"); - - auto uuid_module = py::module_::import("uuid"); - uuid_class = uuid_module.attr("UUID"); - - PyObject* Decimal = decimal_class.ptr(); + PyObject* datetime_module = PyImport_ImportModule("datetime"); + if (!datetime_module) throw py::error_already_set(); + datetime_class = PyObject_GetAttrString(datetime_module, "datetime"); + date_class = PyObject_GetAttrString(datetime_module, "date"); + time_class = PyObject_GetAttrString(datetime_module, "time"); + Py_DECREF(datetime_module); + if (!datetime_class || !date_class || !time_class) throw py::error_already_set(); + + PyObject* decimal_module = PyImport_ImportModule("decimal"); + if (!decimal_module) throw py::error_already_set(); + decimal_class = PyObject_GetAttrString(decimal_module, "Decimal"); + Py_DECREF(decimal_module); + if (!decimal_class) throw py::error_already_set(); + + PyObject* uuid_module = PyImport_ImportModule("uuid"); + if (!uuid_module) throw py::error_already_set(); + uuid_class = PyObject_GetAttrString(uuid_module, "UUID"); + Py_DECREF(uuid_module); + if (!uuid_class) throw py::error_already_set(); + + PyObject* Decimal = decimal_class; smallmoney_min = PyObject_CallFunction(Decimal, "s", "-214748.3648"); smallmoney_max = PyObject_CallFunction(Decimal, "s", "214748.3647"); money_min = PyObject_CallFunction(Decimal, "s", "-922337203685477.5808"); @@ -165,39 +177,97 @@ void initialize() { } } -py::object get_datetime_class() { +static py::object wrap_cached_or_imported(PyObject* obj) { + if (!obj) throw py::error_already_set(); + // Cached path returns a borrowed module-lifetime singleton. Fallback import path + // returns a fresh reference, so steal it to avoid leaking the rare edge-case object. + return cache_initialized ? py::reinterpret_borrow(py::handle(obj)) + : py::reinterpret_steal(obj); +} + +// Returns cached type. Fallback import only for edge case where cache wasn't initialized +// (e.g., called from legacy path before any fast-path execute). +PyObject* get_datetime_class() { if (cache_initialized && datetime_class) { return datetime_class; } - return py::module_::import("datetime").attr("datetime"); + PyObject* mod = PyImport_ImportModule("datetime"); + if (!mod) return nullptr; + PyObject* cls = PyObject_GetAttrString(mod, "datetime"); + Py_DECREF(mod); + return cls; // caller must check for NULL +} + +py::object get_datetime_class_obj() { + return wrap_cached_or_imported(get_datetime_class()); } -py::object get_date_class() { +// Returns cached type. Fallback import only for edge case where cache wasn't initialized +// (e.g., called from legacy path before any fast-path execute). +PyObject* get_date_class() { if (cache_initialized && date_class) { return date_class; } - return py::module_::import("datetime").attr("date"); + PyObject* mod = PyImport_ImportModule("datetime"); + if (!mod) return nullptr; + PyObject* cls = PyObject_GetAttrString(mod, "date"); + Py_DECREF(mod); + return cls; // caller must check for NULL } -py::object get_time_class() { +py::object get_date_class_obj() { + return wrap_cached_or_imported(get_date_class()); +} + +// Returns cached type. Fallback import only for edge case where cache wasn't initialized +// (e.g., called from legacy path before any fast-path execute). +PyObject* get_time_class() { if (cache_initialized && time_class) { return time_class; } - return py::module_::import("datetime").attr("time"); + PyObject* mod = PyImport_ImportModule("datetime"); + if (!mod) return nullptr; + PyObject* cls = PyObject_GetAttrString(mod, "time"); + Py_DECREF(mod); + return cls; // caller must check for NULL +} + +py::object get_time_class_obj() { + return wrap_cached_or_imported(get_time_class()); } -py::object get_decimal_class() { +// Returns cached type. Fallback import only for edge case where cache wasn't initialized +// (e.g., called from legacy path before any fast-path execute). +PyObject* get_decimal_class() { if (cache_initialized && decimal_class) { return decimal_class; } - return py::module_::import("decimal").attr("Decimal"); + PyObject* mod = PyImport_ImportModule("decimal"); + if (!mod) return nullptr; + PyObject* cls = PyObject_GetAttrString(mod, "Decimal"); + Py_DECREF(mod); + return cls; // caller must check for NULL +} + +py::object get_decimal_class_obj() { + return wrap_cached_or_imported(get_decimal_class()); } -py::object get_uuid_class() { +// Returns cached type. Fallback import only for edge case where cache wasn't initialized +// (e.g., called from legacy path before any fast-path execute). +PyObject* get_uuid_class() { if (cache_initialized && uuid_class) { return uuid_class; } - return py::module_::import("uuid").attr("UUID"); + PyObject* mod = PyImport_ImportModule("uuid"); + if (!mod) return nullptr; + PyObject* cls = PyObject_GetAttrString(mod, "UUID"); + Py_DECREF(mod); + return cls; // caller must check for NULL +} + +py::object get_uuid_class_obj() { + return wrap_cached_or_imported(get_uuid_class()); } } // namespace PythonObjectCache @@ -215,15 +285,45 @@ py::object get_uuid_class() { #pragma GCC diagnostic ignored "-Wattributes" #endif struct ParamInfo { - SQLSMALLINT inputOutputType; - SQLSMALLINT paramCType; - SQLSMALLINT paramSQLType; - SQLULEN columnSize; - SQLSMALLINT decimalDigits; + SQLSMALLINT inputOutputType = SQL_PARAM_INPUT; + SQLSMALLINT paramCType = SQL_C_DEFAULT; + SQLSMALLINT paramSQLType = SQL_UNKNOWN_TYPE; + SQLULEN columnSize = 0; + SQLSMALLINT decimalDigits = 0; SQLLEN strLenOrInd = 0; // Required for DAE bool isDAE = false; // Indicates if we need to stream - py::object dataPtr; - Py_ssize_t utf16Len = 0; // UTF-16 code unit count for string params + // Holds a strong reference to the Python object for DAE (data-at-execution) streaming. + // Raw pointer + manual Py_INCREF/DECREF avoids pybind11 wrapper overhead in the hot loop. + PyObject* dataPtr = nullptr; + Py_ssize_t utf16Len = 0; // UTF-16 code unit count for string params + + ParamInfo() = default; + ~ParamInfo() { Py_XDECREF(dataPtr); } + ParamInfo(const ParamInfo&) = delete; + ParamInfo& operator=(const ParamInfo&) = delete; + ParamInfo(ParamInfo&& other) noexcept + : inputOutputType(other.inputOutputType), paramCType(other.paramCType), + paramSQLType(other.paramSQLType), columnSize(other.columnSize), + decimalDigits(other.decimalDigits), strLenOrInd(other.strLenOrInd), + isDAE(other.isDAE), dataPtr(other.dataPtr), utf16Len(other.utf16Len) { + other.dataPtr = nullptr; + } + ParamInfo& operator=(ParamInfo&& other) noexcept { + if (this != &other) { + Py_XDECREF(dataPtr); + inputOutputType = other.inputOutputType; + paramCType = other.paramCType; + paramSQLType = other.paramSQLType; + columnSize = other.columnSize; + decimalDigits = other.decimalDigits; + strLenOrInd = other.strLenOrInd; + isDAE = other.isDAE; + dataPtr = other.dataPtr; + utf16Len = other.utf16Len; + other.dataPtr = nullptr; + } + return *this; + } }; #ifdef __GNUC__ #pragma GCC diagnostic pop @@ -525,42 +625,46 @@ static constexpr SQLSMALLINT PARAM_C_TYPE_TEXT = SQL_C_CHAR; #endif // Forward declare NumericData helper used by decimal path -static py::object build_numeric_data(const py::object& decimal_param); +static NumericData build_numeric_data(PyObject* decimal_param); // --------------------------------------------------------------------------- -// DetectParamTypes — C++ type detection for the execute() fast path. +// DetectParamTypes: Raw CPython parameter type detection for the primary execute path. // -// Replaces the Python-side _create_parameter_types_list() loop by doing type -// detection entirely in C++ using raw CPython type checks. +// Design decisions: +// 1. Operates on a COPY of the user's param list (cursor.py does list(actual_params)). +// We mutate it in-place (PyList_SetItem) for types that need pre-processing +// (time→isoformat string, Decimal→NumericData, UUID→bytes_le). +// 2. Uses CPython macros (PyLong_Check, PyDateTime_Check, etc.) instead of pybind11's +// py::isinstance<> for ~3x faster type checks (direct struct field test vs virtual call). +// 3. Integer range detection uses constants — these match the SQL Server +// 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). +// --------------------------------------------------------------------------- // // ORDERING MATTERS: // - bool before int (bool is a subclass of int in Python) // - datetime before date (datetime is a subclass of date) // -// Some types mutate the params list in-place via PyList_SetItem (which -// decrefs the old slot before stealing the new ref): -// - time → normalized to "HH:MM:SS.ffffff" string -// - Decimal in MONEY range → formatted via __format__("f") -// - Decimal (generic) → converted to NumericData struct -// - UUID → replaced with bytes_le -// This is safe because execute() already copies the caller's param list -// (via list(actual_params)) before reaching this function. -// --------------------------------------------------------------------------- -std::vector DetectParamTypes(py::list& params) { +// Takes a raw PyObject* (must be a list). Caller guarantees it's a fresh copy +// (cursor.py does list(actual_params)), so in-place mutation via PyList_SetItem is safe. +std::vector DetectParamTypes(PyObject* params) { PythonObjectCache::initialize(); - const Py_ssize_t n = PyList_GET_SIZE(params.ptr()); + const Py_ssize_t n = PyList_GET_SIZE(params); std::vector infos(n); - PyObject* decimal_type = PythonObjectCache::get_decimal_class().ptr(); - PyObject* uuid_type = PythonObjectCache::get_uuid_class().ptr(); + PyObject* decimal_type = PythonObjectCache::get_decimal_class(); + PyObject* uuid_type = PythonObjectCache::get_uuid_class(); for (Py_ssize_t i = 0; i < n; ++i) { ParamInfo& info = infos[i]; info.inputOutputType = SQL_PARAM_INPUT; info.isDAE = false; - PyObject* obj = PyList_GET_ITEM(params.ptr(), i); + PyObject* obj = PyList_GET_ITEM(params, i); // --- None --- if (obj == Py_None) { @@ -571,7 +675,9 @@ std::vector DetectParamTypes(py::list& params) { continue; } - // --- bool (must check before int, since bool is subclass of int) --- + // bool must be checked before int: in CPython, PyBool_Type is a subclass of + // PyLong_Type, so PyLong_Check(True) returns 1. If we hit the int branch first, + // True→1 instead of BIT. if (PyBool_Check(obj)) { info.paramSQLType = SQL_BIT; info.paramCType = SQL_C_BIT; @@ -637,12 +743,18 @@ std::vector DetectParamTypes(py::list& params) { } } + // Detect whether the string needs wide-char (NVARCHAR) or narrow (VARCHAR) binding. + // PyUnicode_IS_COMPACT_ASCII is a struct field check (O(1)), not a content scan. + // UCS-1 strings with max_char > 127 contain Latin-1 chars → need NVARCHAR. bool is_unicode = (kind > PyUnicode_1BYTE_KIND) || (PyUnicode_IS_COMPACT_ASCII(obj) == 0 && kind == PyUnicode_1BYTE_KIND && PyUnicode_MAX_CHAR_VALUE(obj) > 127); if (utf16_len > MAX_INLINE_CHAR) { + // Strings > 4000 UTF-16 code units exceed SQL Server's inline NVARCHAR(MAX) + // threshold. Switch to data-at-execution (DAE) streaming: ODBC driver pulls + // data in chunks via SQLPutData, avoiding a single massive buffer allocation. // DAE path: match slow-path types exactly. // Non-unicode (ASCII) → SQL_VARCHAR + PARAM_C_TYPE_TEXT // On Linux/macOS PARAM_C_TYPE_TEXT == SQL_C_WCHAR, matching @@ -652,7 +764,8 @@ std::vector DetectParamTypes(py::list& params) { info.isDAE = true; info.columnSize = 0; info.utf16Len = utf16_len; - info.dataPtr = py::reinterpret_borrow(py::handle(obj)); + info.dataPtr = obj; + Py_INCREF(obj); info.paramSQLType = is_unicode ? SQL_WVARCHAR : SQL_VARCHAR; info.paramCType = is_unicode ? SQL_C_WCHAR : PARAM_C_TYPE_TEXT; } else { @@ -686,7 +799,8 @@ std::vector DetectParamTypes(py::list& params) { if (length > MAX_INLINE_BINARY) { info.isDAE = true; info.columnSize = 0; - info.dataPtr = py::reinterpret_borrow(py::handle(obj)); + info.dataPtr = obj; + Py_INCREF(obj); } else { info.columnSize = std::max(length, 1); } @@ -741,8 +855,8 @@ std::vector DetectParamTypes(py::list& params) { Py_ssize_t time_len = PyUnicode_GET_LENGTH(time_str); info.columnSize = std::max(info.columnSize, time_len); // PyList_SetItem (lowercase) decrefs the old slot before stealing the new - // reference, so this is safe even if `params` is shared with the caller. - PyList_SetItem(params.ptr(), i, time_str); + // reference; safe here because cursor.py already passed a fresh list copy. + PyList_SetItem(params, i, time_str); continue; } @@ -801,8 +915,9 @@ std::vector DetectParamTypes(py::list& params) { std::to_string(precision) + "."); } - // SMALLMONEY/MONEY range — bind as formatted VARCHAR string - // to match SQL Server's fixed-point money semantics. + // 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, PythonObjectCache::smallmoney_min, Py_GE); @@ -840,21 +955,22 @@ std::vector DetectParamTypes(py::list& params) { info.decimalDigits = 0; Py_DECREF(digits_obj); Py_DECREF(as_tuple); - PyList_SetItem(params.ptr(), i, formatted); + PyList_SetItem(params, i, formatted); continue; } - // Generic numeric binding via SQL_NUMERIC_STRUCT + // 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. info.paramSQLType = SQL_NUMERIC; info.paramCType = SQL_C_NUMERIC; Py_DECREF(digits_obj); Py_DECREF(as_tuple); - py::object numeric_data = - build_numeric_data(py::reinterpret_borrow(py::handle(obj))); - NumericData nd = numeric_data.cast(); + NumericData nd = build_numeric_data(obj); info.columnSize = nd.precision; info.decimalDigits = nd.scale; - PyList_SetItem(params.ptr(), i, numeric_data.release().ptr()); + // Store NumericData as a Python object in the param list for the binder. + py::object numeric_obj = py::cast(nd); + PyList_SetItem(params, i, numeric_obj.release().ptr()); continue; } @@ -868,7 +984,7 @@ std::vector DetectParamTypes(py::list& params) { info.paramCType = SQL_C_GUID; info.columnSize = 16; info.decimalDigits = 0; - PyList_SetItem(params.ptr(), i, bytes_le); + PyList_SetItem(params, i, bytes_le); continue; } @@ -881,8 +997,11 @@ std::vector DetectParamTypes(py::list& params) { } // Helper: build SQL_NUMERIC_STRUCT from Python Decimal -static py::object build_numeric_data(const py::object& decimal_param) { - PyObject* as_tuple = PyObject_CallMethod(decimal_param.ptr(), "as_tuple", NULL); +// Converts a Python Decimal into a SQL_NUMERIC_STRUCT representation. +// Takes raw PyObject* (must be a Decimal instance). Returns NumericData directly — +// the caller stores it as a Python capsule via PyList_SetItem for the binder to consume. +static NumericData build_numeric_data(PyObject* decimal_param) { + PyObject* as_tuple = PyObject_CallMethod(decimal_param, "as_tuple", NULL); if (!as_tuple) throw py::error_already_set(); PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); @@ -1055,7 +1174,7 @@ static py::object build_numeric_data(const py::object& decimal_param) { } Py_DECREF(val_bytes); - return py::cast(nd); + return nd; } // GH-610: Resolve SQL type for a NULL parameter using per-handle cache. @@ -1405,7 +1524,7 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par break; } case SQL_C_TYPE_DATE: { - py::object dateType = PythonObjectCache::get_date_class(); + py::object dateType = PythonObjectCache::get_date_class_obj(); if (!py::isinstance(param, dateType)) { ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex)); } @@ -1425,7 +1544,7 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par break; } case SQL_C_TYPE_TIME: { - py::object timeType = PythonObjectCache::get_time_class(); + py::object timeType = PythonObjectCache::get_time_class_obj(); if (!py::isinstance(param, timeType)) { ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex)); } @@ -1439,7 +1558,7 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par break; } case SQL_C_SS_TIMESTAMPOFFSET: { - py::object datetimeType = PythonObjectCache::get_datetime_class(); + py::object datetimeType = PythonObjectCache::get_datetime_class_obj(); if (!py::isinstance(param, datetimeType)) { ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex)); } @@ -1491,7 +1610,7 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par break; } case SQL_C_TYPE_TIMESTAMP: { - py::object datetimeType = PythonObjectCache::get_datetime_class(); + py::object datetimeType = PythonObjectCache::get_datetime_class_obj(); if (!py::isinstance(param, datetimeType)) { ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex)); } @@ -2522,14 +2641,15 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u if (!matchedInfo) { ThrowStdException("Unrecognized paramToken returned by SQLParamData"); } - const py::object& pyObj = matchedInfo->dataPtr; - if (pyObj.is_none()) { + PyObject* pyObj = matchedInfo->dataPtr; + if (!pyObj || pyObj == Py_None) { putData(nullptr, 0); continue; } - if (py::isinstance(pyObj)) { + if (PyUnicode_Check(pyObj)) { if (matchedInfo->paramCType == SQL_C_WCHAR) { - std::u16string utf16 = pyObj.cast(); + std::u16string utf16 = + py::reinterpret_borrow(py::handle(pyObj)).cast(); size_t totalChars = utf16.size(); const SQLWCHAR* dataPtr = reinterpretU16stringAsSqlWChar(utf16); size_t offset = 0; @@ -2556,14 +2676,11 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u // Encode the string using the specified encoding std::string encodedStr; try { - if (py::isinstance(pyObj)) { - py::object encoded = pyObj.attr("encode")(charEncoding, "strict"); - encodedStr = encoded.cast(); - LOG("SQLExecute: DAE SQL_C_CHAR - Encoded with '%s', %zu bytes", - charEncoding.c_str(), encodedStr.size()); - } else { - encodedStr = pyObj.cast(); - } + py::object encoded = py::reinterpret_borrow(py::handle(pyObj)) + .attr("encode")(charEncoding, "strict"); + encodedStr = encoded.cast(); + LOG("SQLExecute: DAE SQL_C_CHAR - Encoded with '%s', %zu bytes", + charEncoding.c_str(), encodedStr.size()); } catch (const py::error_already_set& e) { LOG_ERROR("SQLExecute: DAE SQL_C_CHAR - Failed to encode with '%s': %s", charEncoding.c_str(), e.what()); @@ -2589,12 +2706,18 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u } else { ThrowStdException("Unsupported C type for str in DAE"); } - } else if (py::isinstance(pyObj) || - py::isinstance(pyObj)) { - py::bytes b = pyObj.cast(); - std::string s = b; - const char* dataPtr = s.data(); - size_t totalBytes = s.size(); + } else if (PyBytes_Check(pyObj) || PyByteArray_Check(pyObj)) { + const char* dataPtr = nullptr; + size_t totalBytes = 0; + std::string bytesStorage; // lifetime must span the loop + if (PyBytes_Check(pyObj)) { + bytesStorage = py::reinterpret_borrow(py::handle(pyObj)); + dataPtr = bytesStorage.data(); + totalBytes = bytesStorage.size(); + } else { + dataPtr = PyByteArray_AS_STRING(pyObj); + totalBytes = static_cast(PyByteArray_GET_SIZE(pyObj)); + } const size_t chunkSize = DAE_CHUNK_SIZE; for (size_t offset = 0; offset < totalBytes; offset += chunkSize) { size_t len = std::min(chunkSize, totalBytes - offset); @@ -2685,7 +2808,7 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, // Run DetectParamTypes BEFORE SQLPrepare so that type-detection errors // (unsupported type, NaN Decimal, precision overflow) don't leave the // cursor in a half-prepared state. - std::vector paramInfos = DetectParamTypes(params); + std::vector paramInfos = DetectParamTypes(params.ptr()); RETCODE rc; bool already_prepared = is_stmt_prepared[0].cast(); @@ -2741,16 +2864,17 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, if (!matchedInfo) { ThrowStdException("SQLExecuteFast: unrecognized paramToken from SQLParamData"); } - const py::object& pyObj = matchedInfo->dataPtr; - if (pyObj.is_none()) { + PyObject* pyObj = matchedInfo->dataPtr; + if (!pyObj || pyObj == Py_None) { py::gil_scoped_release release; SQLPutData_ptr(hStmt, nullptr, 0); continue; } - if (py::isinstance(pyObj)) { + if (PyUnicode_Check(pyObj)) { if (matchedInfo->paramCType == SQL_C_WCHAR) { - std::u16string u16 = pyObj.cast(); + std::u16string u16 = + py::reinterpret_borrow(py::handle(pyObj)).cast(); const SQLWCHAR* dataPtr = reinterpretU16stringAsSqlWChar(u16); size_t totalChars = u16.size(); size_t chunkChars = DAE_CHUNK_SIZE / sizeof(SQLWCHAR); @@ -2765,7 +2889,8 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, } } else if (matchedInfo->paramCType == SQL_C_CHAR) { std::string encodedStr; - py::object encoded = pyObj.attr("encode")(charEncoding, "strict"); + py::object encoded = py::reinterpret_borrow(py::handle(pyObj)) + .attr("encode")(charEncoding, "strict"); encodedStr = encoded.cast(); const char* dataPtr = encodedStr.data(); size_t totalBytes = encodedStr.size(); @@ -2782,23 +2907,21 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, } else { ThrowStdException("SQLExecuteFast: unsupported C type for str in DAE"); } - } else if (py::isinstance(pyObj) || - py::isinstance(pyObj)) { + } else if (PyBytes_Check(pyObj) || PyByteArray_Check(pyObj)) { // Handle bytes and bytearray separately — pybind11's bytes // caster does not safely convert bytearray. const char* dataPtr = nullptr; size_t totalBytes = 0; std::string bytesStorage; // lifetime must span the loop - if (py::isinstance(pyObj)) { - bytesStorage = pyObj.cast(); + if (PyBytes_Check(pyObj)) { + bytesStorage = py::reinterpret_borrow(py::handle(pyObj)); dataPtr = bytesStorage.data(); totalBytes = bytesStorage.size(); } else { // bytearray: use raw buffer access - PyObject* ba = pyObj.ptr(); - dataPtr = PyByteArray_AS_STRING(ba); - totalBytes = static_cast(PyByteArray_GET_SIZE(ba)); + dataPtr = PyByteArray_AS_STRING(pyObj); + totalBytes = static_cast(PyByteArray_GET_SIZE(pyObj)); } for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { @@ -3251,7 +3374,7 @@ SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list& AllocateParamBufferArray(tempBuffers, paramSetSize); strLenOrIndArray = AllocateParamBufferArray(tempBuffers, paramSetSize); - py::object datetimeType = PythonObjectCache::get_datetime_class(); + py::object datetimeType = PythonObjectCache::get_datetime_class_obj(); for (size_t i = 0; i < paramSetSize; ++i) { const py::handle& param = columnValues[i]; @@ -3366,7 +3489,7 @@ SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list& // Get cached UUID class from module-level helper // This avoids static object destruction issues during // Python finalization - py::object uuid_class = PythonObjectCache::get_uuid_class(); + py::object uuid_class = PythonObjectCache::get_uuid_class_obj(); // Get cached UUID class for (size_t i = 0; i < paramSetSize; ++i) { @@ -4333,7 +4456,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p // parsing The decimal separator only affects display // formatting, not parsing py::object decimalObj = - PythonObjectCache::get_decimal_class()(py::str(cnum, safeLen)); + PythonObjectCache::get_decimal_class_obj()(py::str(cnum, safeLen)); row.append(decimalObj); } catch (const py::error_already_set& e) { // If conversion fails, append None @@ -4383,7 +4506,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_DATE, &dateValue, sizeof(dateValue), NULL); if (SQL_SUCCEEDED(ret)) { - row.append(PythonObjectCache::get_date_class()(dateValue.year, dateValue.month, + row.append(PythonObjectCache::get_date_class_obj()(dateValue.year, dateValue.month, dateValue.day)); } else { row.append(py::none()); @@ -4396,7 +4519,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p SQLLEN indicator = 0; ret = SQLGetData_ptr(hStmt, i, SQL_C_SS_TIME2, &t2, sizeof(t2), &indicator); if (SQL_SUCCEEDED(ret) && indicator != SQL_NULL_DATA) { - row.append(PythonObjectCache::get_time_class()( + row.append(PythonObjectCache::get_time_class_obj()( t2.hour, t2.minute, t2.second, t2.fraction / 1000)); // ns to µs } else { if (!SQL_SUCCEEDED(ret)) { @@ -4415,7 +4538,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p ret = SQLGetData_ptr(hStmt, i, SQL_C_TYPE_TIMESTAMP, ×tampValue, sizeof(timestampValue), NULL); if (SQL_SUCCEEDED(ret)) { - row.append(PythonObjectCache::get_datetime_class()( + row.append(PythonObjectCache::get_datetime_class_obj()( timestampValue.year, timestampValue.month, timestampValue.day, timestampValue.hour, timestampValue.minute, timestampValue.second, timestampValue.fraction / 1000 // Convert back ns to µs @@ -4455,7 +4578,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p py::object datetime_module = py::module_::import("datetime"); py::object tzinfo = datetime_module.attr("timezone")( datetime_module.attr("timedelta")(py::arg("minutes") = totalMinutes)); - py::object py_dt = PythonObjectCache::get_datetime_class()( + py::object py_dt = PythonObjectCache::get_datetime_class_obj()( dtoValue.year, dtoValue.month, dtoValue.day, dtoValue.hour, dtoValue.minute, dtoValue.second, microseconds, tzinfo); row.append(py_dt); @@ -4562,7 +4685,7 @@ SQLRETURN SQLGetData_wrap(SqlHandlePtr StatementHandle, SQLUSMALLINT colCount, p py::bytes py_guid_bytes(guid_bytes.data(), guid_bytes.size()); py::object uuid_obj = - PythonObjectCache::get_uuid_class()(py::arg("bytes") = py_guid_bytes); + PythonObjectCache::get_uuid_class_obj()(py::arg("bytes") = py_guid_bytes); row.append(uuid_obj); } else if (indicator == SQL_NULL_DATA) { row.append(py::none()); @@ -5025,7 +5148,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum // parsing The decimal separator only affects display // formatting, not parsing PyObject* decimalObj = - PythonObjectCache::get_decimal_class()(py::str(rawData, decimalDataLen)) + PythonObjectCache::get_decimal_class_obj()(py::str(rawData, decimalDataLen)) .release() .ptr(); PyList_SET_ITEM(row, col - 1, decimalObj); @@ -5042,7 +5165,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum case SQL_TYPE_TIMESTAMP: case SQL_DATETIME: { const SQL_TIMESTAMP_STRUCT& ts = buffers.timestampBuffers[col - 1][i]; - PyObject* datetimeObj = PythonObjectCache::get_datetime_class()( + PyObject* datetimeObj = PythonObjectCache::get_datetime_class_obj()( ts.year, ts.month, ts.day, ts.hour, ts.minute, ts.second, ts.fraction / 1000) .release() @@ -5052,7 +5175,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum } case SQL_TYPE_DATE: { PyObject* dateObj = - PythonObjectCache::get_date_class()(buffers.dateBuffers[col - 1][i].year, + PythonObjectCache::get_date_class_obj()(buffers.dateBuffers[col - 1][i].year, buffers.dateBuffers[col - 1][i].month, buffers.dateBuffers[col - 1][i].day) .release() @@ -5063,7 +5186,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum case SQL_SS_TIME2: { const SQL_SS_TIME2_STRUCT& t2 = buffers.timeBuffers[col - 1][i]; PyObject* timeObj = - PythonObjectCache::get_time_class()(t2.hour, t2.minute, t2.second, + PythonObjectCache::get_time_class_obj()(t2.hour, t2.minute, t2.second, t2.fraction / 1000) // ns to µs .release() .ptr(); @@ -5079,7 +5202,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum py::object datetime_module = py::module_::import("datetime"); py::object tzinfo = datetime_module.attr("timezone")( datetime_module.attr("timedelta")(py::arg("minutes") = totalMinutes)); - py::object py_dt = PythonObjectCache::get_datetime_class()( + py::object py_dt = PythonObjectCache::get_datetime_class_obj()( dtoValue.year, dtoValue.month, dtoValue.day, dtoValue.hour, dtoValue.minute, dtoValue.second, dtoValue.fraction / 1000, // ns → µs @@ -5113,7 +5236,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum py::bytes py_guid_bytes(reinterpret_cast(reordered), 16); py::dict kwargs; kwargs["bytes"] = py_guid_bytes; - py::object uuid_obj = PythonObjectCache::get_uuid_class()(**kwargs); + py::object uuid_obj = PythonObjectCache::get_uuid_class_obj()(**kwargs); PyList_SET_ITEM(row, col - 1, uuid_obj.release().ptr()); break; } @@ -6660,7 +6783,19 @@ PYBIND11_MODULE(ddbc_bindings, m) { .def_readwrite("columnSize", &ParamInfo::columnSize) .def_readwrite("decimalDigits", &ParamInfo::decimalDigits) .def_readwrite("strLenOrInd", &ParamInfo::strLenOrInd) - .def_readwrite("dataPtr", &ParamInfo::dataPtr) + .def_property( + "dataPtr", + [](const ParamInfo& info) -> py::object { + if (!info.dataPtr) { + return py::none(); + } + return py::reinterpret_borrow(py::handle(info.dataPtr)); + }, + [](ParamInfo& info, py::object obj) { + Py_XINCREF(obj.ptr()); + Py_XDECREF(info.dataPtr); + info.dataPtr = obj.ptr(); + }) .def_readwrite("isDAE", &ParamInfo::isDAE); // Define numeric data class From 3d73824bb5cd224693ba2cac6a645f113ed9977a Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 12:06:48 +0530 Subject: [PATCH 14/23] FIX: Add ParamInfo copy constructor for pybind11 legacy path compatibility pybind11's type_caster needs copy semantics for std::vector& in the legacy path. Provide a copy ctor that Py_XINCREFs dataPtr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 4681c1dd..12ea2312 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -299,8 +299,30 @@ struct ParamInfo { ParamInfo() = default; ~ParamInfo() { Py_XDECREF(dataPtr); } - ParamInfo(const ParamInfo&) = delete; - ParamInfo& operator=(const ParamInfo&) = delete; + // Copy needed by pybind11 type_caster for the legacy path (std::vector&). + ParamInfo(const ParamInfo& other) + : inputOutputType(other.inputOutputType), paramCType(other.paramCType), + paramSQLType(other.paramSQLType), columnSize(other.columnSize), + decimalDigits(other.decimalDigits), strLenOrInd(other.strLenOrInd), + isDAE(other.isDAE), dataPtr(other.dataPtr), utf16Len(other.utf16Len) { + Py_XINCREF(dataPtr); + } + ParamInfo& operator=(const ParamInfo& other) { + if (this != &other) { + Py_XDECREF(dataPtr); + inputOutputType = other.inputOutputType; + paramCType = other.paramCType; + paramSQLType = other.paramSQLType; + columnSize = other.columnSize; + decimalDigits = other.decimalDigits; + strLenOrInd = other.strLenOrInd; + isDAE = other.isDAE; + dataPtr = other.dataPtr; + utf16Len = other.utf16Len; + Py_XINCREF(dataPtr); + } + return *this; + } ParamInfo(ParamInfo&& other) noexcept : inputOutputType(other.inputOutputType), paramCType(other.paramCType), paramSQLType(other.paramSQLType), columnSize(other.columnSize), From 98eb2549fca84bdc825b5bf713f07aef0023aca4 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 12:41:46 +0530 Subject: [PATCH 15/23] FIX: Use explicit byte length for string binding instead of SQL_NTS Strings with embedded NUL characters (e.g., 'hello\x00world') were truncated at the first NUL because BindParameters used SQL_NTS (null-terminated string indicator). Now passes the actual byte/char length so ODBC sees the full string. Fixes test_string_with_embedded_nulls on all platforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 12ea2312..f965aa2b 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1371,10 +1371,12 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par std::string* strParam = AllocateParamBuffer(paramBuffers, encodedStr); - dataPtr = const_cast(static_cast(strParam->c_str())); - bufferLength = strParam->size() + 1; + dataPtr = const_cast(static_cast(strParam->data())); + bufferLength = strParam->size(); strLenOrIndPtr = AllocateParamBuffer(paramBuffers); - *strLenOrIndPtr = SQL_NTS; + // Use explicit byte length instead of SQL_NTS so embedded NUL chars + // aren't treated as string terminators (e.g., "hello\x00world"). + *strLenOrIndPtr = static_cast(strParam->size()); } break; } @@ -1439,7 +1441,9 @@ SQLRETURN BindParameters(SqlHandle& handle, SQLHANDLE hStmt, const py::list& par dataPtr = sqlwcharBuffer->data(); bufferLength = sqlwcharBuffer->size() * sizeof(SQLWCHAR); strLenOrIndPtr = AllocateParamBuffer(paramBuffers); - *strLenOrIndPtr = SQL_NTS; + // Use explicit byte length instead of SQL_NTS so embedded NUL chars + // aren't treated as string terminators. + *strLenOrIndPtr = static_cast(sqlwcharBuffer->size() * sizeof(SQLWCHAR)); } break; } From 4ee2db95369fc3b28c9947d22d7284fd28be3b4c Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 15:49:43 +0530 Subject: [PATCH 16/23] TEST: Add edge-case tests and LCOV exclusions for untestable paths - Add tests: integer overflow (2**63), Decimal NaN/sNaN, precision > 38 - Add LCOV_EXCL markers on CPython import-failure and cache-fallback paths - Add contextual comments on PythonObjectCache and ParamInfo operators Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 55 ++++++++++++++++++++------- tests/test_023_fast_path_parity.py | 22 +++++++++++ 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index f965aa2b..37f99357 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -129,39 +129,53 @@ static PyObject* smallmoney_min = nullptr; static PyObject* smallmoney_max = nullptr; static bool cache_initialized = false; +// One-time cache of Python type objects and MONEY boundary constants. +// Called on first execute(). Uses raw CPython API (not pybind11) because +// these cached PyObject* are compared via PyObject_IsInstance in the +// hot DetectParamTypes loop — wrapping them in py::object would add +// unnecessary ref-count traffic on every parameter. void initialize() { if (!cache_initialized) { PyDateTime_IMPORT; if (PyDateTimeAPI == nullptr) { - throw py::error_already_set(); + throw py::error_already_set(); // LCOV_EXCL_LINE } + // Cache datetime.datetime, datetime.date, datetime.time for isinstance + // checks in DetectParamTypes. Single import, then borrowed refs held + // for process lifetime (module-level statics, never decremented). PyObject* datetime_module = PyImport_ImportModule("datetime"); - if (!datetime_module) throw py::error_already_set(); + if (!datetime_module) throw py::error_already_set(); // LCOV_EXCL_LINE datetime_class = PyObject_GetAttrString(datetime_module, "datetime"); date_class = PyObject_GetAttrString(datetime_module, "date"); time_class = PyObject_GetAttrString(datetime_module, "time"); Py_DECREF(datetime_module); - if (!datetime_class || !date_class || !time_class) throw py::error_already_set(); + if (!datetime_class || !date_class || !time_class) + throw py::error_already_set(); // LCOV_EXCL_LINE PyObject* decimal_module = PyImport_ImportModule("decimal"); - if (!decimal_module) throw py::error_already_set(); + if (!decimal_module) throw py::error_already_set(); // LCOV_EXCL_LINE decimal_class = PyObject_GetAttrString(decimal_module, "Decimal"); Py_DECREF(decimal_module); - if (!decimal_class) throw py::error_already_set(); + if (!decimal_class) throw py::error_already_set(); // LCOV_EXCL_LINE PyObject* uuid_module = PyImport_ImportModule("uuid"); - if (!uuid_module) throw py::error_already_set(); + if (!uuid_module) throw py::error_already_set(); // LCOV_EXCL_LINE uuid_class = PyObject_GetAttrString(uuid_module, "UUID"); Py_DECREF(uuid_module); - if (!uuid_class) throw py::error_already_set(); + if (!uuid_class) throw py::error_already_set(); // LCOV_EXCL_LINE + // Pre-compute MONEY/SMALLMONEY boundary Decimals once. DetectParamTypes + // uses PyObject_RichCompareBool against these to classify Decimal params + // into MONEY vs NUMERIC — doing exact Decimal comparison (not float cast) + // avoids boundary misclassification at the edges. PyObject* Decimal = decimal_class; smallmoney_min = PyObject_CallFunction(Decimal, "s", "-214748.3648"); smallmoney_max = PyObject_CallFunction(Decimal, "s", "214748.3647"); money_min = PyObject_CallFunction(Decimal, "s", "-922337203685477.5808"); money_max = PyObject_CallFunction(Decimal, "s", "922337203685477.5807"); if (!smallmoney_min || !smallmoney_max || !money_min || !money_max) { + // Partial creation — clean up whichever succeeded before throwing. Py_XDECREF(smallmoney_min); Py_XDECREF(smallmoney_max); Py_XDECREF(money_min); @@ -170,7 +184,7 @@ void initialize() { smallmoney_max = nullptr; money_min = nullptr; money_max = nullptr; - throw py::error_already_set(); + throw py::error_already_set(); // LCOV_EXCL_LINE } cache_initialized = true; @@ -191,11 +205,13 @@ PyObject* get_datetime_class() { if (cache_initialized && datetime_class) { return datetime_class; } + // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("datetime"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "datetime"); Py_DECREF(mod); return cls; // caller must check for NULL + // LCOV_EXCL_STOP } py::object get_datetime_class_obj() { @@ -208,11 +224,13 @@ PyObject* get_date_class() { if (cache_initialized && date_class) { return date_class; } + // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("datetime"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "date"); Py_DECREF(mod); return cls; // caller must check for NULL + // LCOV_EXCL_STOP } py::object get_date_class_obj() { @@ -225,11 +243,13 @@ PyObject* get_time_class() { if (cache_initialized && time_class) { return time_class; } + // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("datetime"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "time"); Py_DECREF(mod); return cls; // caller must check for NULL + // LCOV_EXCL_STOP } py::object get_time_class_obj() { @@ -242,11 +262,13 @@ PyObject* get_decimal_class() { if (cache_initialized && decimal_class) { return decimal_class; } + // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("decimal"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "Decimal"); Py_DECREF(mod); return cls; // caller must check for NULL + // LCOV_EXCL_STOP } py::object get_decimal_class_obj() { @@ -259,11 +281,13 @@ PyObject* get_uuid_class() { if (cache_initialized && uuid_class) { return uuid_class; } + // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("uuid"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "UUID"); Py_DECREF(mod); return cls; // caller must check for NULL + // LCOV_EXCL_STOP } py::object get_uuid_class_obj() { @@ -307,6 +331,10 @@ struct ParamInfo { isDAE(other.isDAE), dataPtr(other.dataPtr), utf16Len(other.utf16Len) { Py_XINCREF(dataPtr); } + // Copy/move assignment and move constructor below are required for + // std::vector resize/reallocation and pybind11's type_caster. + // They manually manage dataPtr refcounts since ParamInfo owns a strong ref. + // LCOV_EXCL_START — exercised by STL internals, not directly testable. ParamInfo& operator=(const ParamInfo& other) { if (this != &other) { Py_XDECREF(dataPtr); @@ -346,6 +374,7 @@ struct ParamInfo { } return *this; } + // LCOV_EXCL_STOP }; #ifdef __GNUC__ #pragma GCC diagnostic pop @@ -892,7 +921,7 @@ std::vector DetectParamTypes(PyObject* params) { PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); if (!exponent_obj) { Py_DECREF(as_tuple); - throw py::error_already_set(); + throw py::error_already_set(); // LCOV_EXCL_LINE } // NaN / Infinity / sNaN: refuse rather than silently writing 0. @@ -907,7 +936,7 @@ std::vector DetectParamTypes(PyObject* params) { if (!digits_obj) { Py_DECREF(exponent_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); + throw py::error_already_set(); // LCOV_EXCL_LINE } Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj); @@ -916,7 +945,7 @@ std::vector DetectParamTypes(PyObject* params) { if (exponent == -1 && PyErr_Occurred()) { Py_DECREF(digits_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); + throw py::error_already_set(); // LCOV_EXCL_LINE } int precision; @@ -947,7 +976,7 @@ std::vector DetectParamTypes(PyObject* params) { if (cmp_ge == -1 || cmp_le == -1) { Py_DECREF(digits_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); + throw py::error_already_set(); // LCOV_EXCL_LINE } if (cmp_ge == 1 && cmp_le == 1) { in_money_range = true; @@ -957,7 +986,7 @@ std::vector DetectParamTypes(PyObject* params) { if (cmp_ge == -1 || cmp_le == -1) { Py_DECREF(digits_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); + throw py::error_already_set(); // LCOV_EXCL_LINE } if (cmp_ge == 1 && cmp_le == 1) { in_money_range = true; diff --git a/tests/test_023_fast_path_parity.py b/tests/test_023_fast_path_parity.py index c4f2b383..77272caf 100644 --- a/tests/test_023_fast_path_parity.py +++ b/tests/test_023_fast_path_parity.py @@ -280,3 +280,25 @@ def test_string_with_embedded_nulls(cursor): cursor.execute("SELECT LEN(?)", [value]) result = cursor.fetchone()[0] assert result == 11 + + +def test_integer_overflow_detected(cursor): + """Integers beyond int64 range trigger overflow detection in C++ and bind as BIGINT. + SQL Server cannot store values outside [-2^63, 2^63-1] in BIGINT, so these raise.""" + with pytest.raises(Exception): + cursor.execute("SELECT ?", [2**63]) + with pytest.raises(Exception): + cursor.execute("SELECT ?", [-(2**63) - 1]) + + +@pytest.mark.parametrize("value", [decimal.Decimal("NaN"), decimal.Decimal("sNaN")]) +def test_decimal_nan_variants_rejected(cursor, value): + """Decimal NaN variants must raise, not silently bind as 0.""" + with pytest.raises(Exception): + cursor.execute("SELECT ?", [value]) + + +def test_decimal_precision_overflow_rejected(cursor): + """Decimals beyond SQL Server's max precision must raise.""" + with pytest.raises(Exception): + cursor.execute("SELECT ?", [decimal.Decimal("123456789012345678901234567890123456789")]) From 8941070be702f9617c2a96eda3a9d7dc4bed1f6a Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 20:06:09 +0530 Subject: [PATCH 17/23] =?UTF-8?q?FIX:=20Address=20review=20comments=20?= =?UTF-8?q?=E2=80=94=20PyList=5FSetItem=20checks=20and=20bytearray=20DAE?= =?UTF-8?q?=20safety?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Check PyList_SetItem return value at all 4 call sites in DetectParamTypes - Copy mutable bytearray into std::string before DAE streaming (both paths) - Revert LCOV_EXCL markers (not processed by llvm-cov pipeline) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 72 ++++++++++++++------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 37f99357..49b5a15f 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -138,32 +138,32 @@ void initialize() { if (!cache_initialized) { PyDateTime_IMPORT; if (PyDateTimeAPI == nullptr) { - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); } // Cache datetime.datetime, datetime.date, datetime.time for isinstance // checks in DetectParamTypes. Single import, then borrowed refs held // for process lifetime (module-level statics, never decremented). PyObject* datetime_module = PyImport_ImportModule("datetime"); - if (!datetime_module) throw py::error_already_set(); // LCOV_EXCL_LINE + if (!datetime_module) throw py::error_already_set(); datetime_class = PyObject_GetAttrString(datetime_module, "datetime"); date_class = PyObject_GetAttrString(datetime_module, "date"); time_class = PyObject_GetAttrString(datetime_module, "time"); Py_DECREF(datetime_module); if (!datetime_class || !date_class || !time_class) - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); PyObject* decimal_module = PyImport_ImportModule("decimal"); - if (!decimal_module) throw py::error_already_set(); // LCOV_EXCL_LINE + if (!decimal_module) throw py::error_already_set(); decimal_class = PyObject_GetAttrString(decimal_module, "Decimal"); Py_DECREF(decimal_module); - if (!decimal_class) throw py::error_already_set(); // LCOV_EXCL_LINE + if (!decimal_class) throw py::error_already_set(); PyObject* uuid_module = PyImport_ImportModule("uuid"); - if (!uuid_module) throw py::error_already_set(); // LCOV_EXCL_LINE + if (!uuid_module) throw py::error_already_set(); uuid_class = PyObject_GetAttrString(uuid_module, "UUID"); Py_DECREF(uuid_module); - if (!uuid_class) throw py::error_already_set(); // LCOV_EXCL_LINE + if (!uuid_class) throw py::error_already_set(); // Pre-compute MONEY/SMALLMONEY boundary Decimals once. DetectParamTypes // uses PyObject_RichCompareBool against these to classify Decimal params @@ -184,7 +184,7 @@ void initialize() { smallmoney_max = nullptr; money_min = nullptr; money_max = nullptr; - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); } cache_initialized = true; @@ -205,13 +205,11 @@ PyObject* get_datetime_class() { if (cache_initialized && datetime_class) { return datetime_class; } - // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("datetime"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "datetime"); Py_DECREF(mod); return cls; // caller must check for NULL - // LCOV_EXCL_STOP } py::object get_datetime_class_obj() { @@ -224,13 +222,11 @@ PyObject* get_date_class() { if (cache_initialized && date_class) { return date_class; } - // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("datetime"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "date"); Py_DECREF(mod); return cls; // caller must check for NULL - // LCOV_EXCL_STOP } py::object get_date_class_obj() { @@ -243,13 +239,11 @@ PyObject* get_time_class() { if (cache_initialized && time_class) { return time_class; } - // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("datetime"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "time"); Py_DECREF(mod); return cls; // caller must check for NULL - // LCOV_EXCL_STOP } py::object get_time_class_obj() { @@ -262,13 +256,11 @@ PyObject* get_decimal_class() { if (cache_initialized && decimal_class) { return decimal_class; } - // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("decimal"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "Decimal"); Py_DECREF(mod); return cls; // caller must check for NULL - // LCOV_EXCL_STOP } py::object get_decimal_class_obj() { @@ -281,13 +273,11 @@ PyObject* get_uuid_class() { if (cache_initialized && uuid_class) { return uuid_class; } - // LCOV_EXCL_START PyObject* mod = PyImport_ImportModule("uuid"); if (!mod) return nullptr; PyObject* cls = PyObject_GetAttrString(mod, "UUID"); Py_DECREF(mod); return cls; // caller must check for NULL - // LCOV_EXCL_STOP } py::object get_uuid_class_obj() { @@ -334,7 +324,6 @@ struct ParamInfo { // Copy/move assignment and move constructor below are required for // std::vector resize/reallocation and pybind11's type_caster. // They manually manage dataPtr refcounts since ParamInfo owns a strong ref. - // LCOV_EXCL_START — exercised by STL internals, not directly testable. ParamInfo& operator=(const ParamInfo& other) { if (this != &other) { Py_XDECREF(dataPtr); @@ -374,7 +363,6 @@ struct ParamInfo { } return *this; } - // LCOV_EXCL_STOP }; #ifdef __GNUC__ #pragma GCC diagnostic pop @@ -907,7 +895,9 @@ std::vector DetectParamTypes(PyObject* params) { info.columnSize = std::max(info.columnSize, time_len); // PyList_SetItem (lowercase) decrefs the old slot before stealing the new // reference; safe here because cursor.py already passed a fresh list copy. - PyList_SetItem(params, i, time_str); + if (PyList_SetItem(params, i, time_str) != 0) { + throw py::error_already_set(); + } continue; } @@ -921,7 +911,7 @@ std::vector DetectParamTypes(PyObject* params) { PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); if (!exponent_obj) { Py_DECREF(as_tuple); - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); } // NaN / Infinity / sNaN: refuse rather than silently writing 0. @@ -936,7 +926,7 @@ std::vector DetectParamTypes(PyObject* params) { if (!digits_obj) { Py_DECREF(exponent_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); } Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj); @@ -945,7 +935,7 @@ std::vector DetectParamTypes(PyObject* params) { if (exponent == -1 && PyErr_Occurred()) { Py_DECREF(digits_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); } int precision; @@ -976,7 +966,7 @@ std::vector DetectParamTypes(PyObject* params) { if (cmp_ge == -1 || cmp_le == -1) { Py_DECREF(digits_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); } if (cmp_ge == 1 && cmp_le == 1) { in_money_range = true; @@ -986,7 +976,7 @@ std::vector DetectParamTypes(PyObject* params) { if (cmp_ge == -1 || cmp_le == -1) { Py_DECREF(digits_obj); Py_DECREF(as_tuple); - throw py::error_already_set(); // LCOV_EXCL_LINE + throw py::error_already_set(); } if (cmp_ge == 1 && cmp_le == 1) { in_money_range = true; @@ -1006,7 +996,9 @@ std::vector DetectParamTypes(PyObject* params) { info.decimalDigits = 0; Py_DECREF(digits_obj); Py_DECREF(as_tuple); - PyList_SetItem(params, i, formatted); + if (PyList_SetItem(params, i, formatted) != 0) { + throw py::error_already_set(); + } continue; } @@ -1021,7 +1013,11 @@ std::vector DetectParamTypes(PyObject* params) { info.decimalDigits = nd.scale; // Store NumericData as a Python object in the param list for the binder. py::object numeric_obj = py::cast(nd); - PyList_SetItem(params, i, numeric_obj.release().ptr()); + PyObject* raw = numeric_obj.release().ptr(); + if (PyList_SetItem(params, i, raw) != 0) { + Py_DECREF(raw); + throw py::error_already_set(); + } continue; } @@ -1035,7 +1031,10 @@ std::vector DetectParamTypes(PyObject* params) { info.paramCType = SQL_C_GUID; info.columnSize = 16; info.decimalDigits = 0; - PyList_SetItem(params, i, bytes_le); + if (PyList_SetItem(params, i, bytes_le) != 0) { + Py_DECREF(bytes_le); + throw py::error_already_set(); + } continue; } @@ -2770,8 +2769,11 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u dataPtr = bytesStorage.data(); totalBytes = bytesStorage.size(); } else { - dataPtr = PyByteArray_AS_STRING(pyObj); - totalBytes = static_cast(PyByteArray_GET_SIZE(pyObj)); + // bytearray is mutable — copy to stable buffer before streaming + bytesStorage.assign(PyByteArray_AS_STRING(pyObj), + static_cast(PyByteArray_GET_SIZE(pyObj))); + dataPtr = bytesStorage.data(); + totalBytes = bytesStorage.size(); } const size_t chunkSize = DAE_CHUNK_SIZE; for (size_t offset = 0; offset < totalBytes; offset += chunkSize) { @@ -2974,9 +2976,11 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, dataPtr = bytesStorage.data(); totalBytes = bytesStorage.size(); } else { - // bytearray: use raw buffer access - dataPtr = PyByteArray_AS_STRING(pyObj); - totalBytes = static_cast(PyByteArray_GET_SIZE(pyObj)); + // bytearray is mutable — copy to stable buffer before streaming + bytesStorage.assign(PyByteArray_AS_STRING(pyObj), + static_cast(PyByteArray_GET_SIZE(pyObj))); + dataPtr = bytesStorage.data(); + totalBytes = bytesStorage.size(); } for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { From 432b52278fcb57d62ad2a4495381e75ba65b98b0 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 14 Jul 2026 21:17:02 +0530 Subject: [PATCH 18/23] FIX: Harden Decimal subclass safety, RAII param cleanup, cache init leak - Add PyTuple_Check guard before PyTuple_GET_SIZE/GET_ITEM on Decimal.as_tuple().digits in DetectParamTypes and build_numeric_data - Add ParamResetGuard RAII struct to ensure SQLFreeStmt(SQL_RESET_PARAMS) fires on all exit paths after BindParameters succeeds - Wrap PythonObjectCache::initialize() in try/catch to clean up partial refs on any import failure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 133 +++++++++++++++++--------- 1 file changed, 86 insertions(+), 47 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 49b5a15f..37f2eb69 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -141,53 +141,67 @@ void initialize() { throw py::error_already_set(); } - // Cache datetime.datetime, datetime.date, datetime.time for isinstance - // checks in DetectParamTypes. Single import, then borrowed refs held - // for process lifetime (module-level statics, never decremented). - PyObject* datetime_module = PyImport_ImportModule("datetime"); - if (!datetime_module) throw py::error_already_set(); - datetime_class = PyObject_GetAttrString(datetime_module, "datetime"); - date_class = PyObject_GetAttrString(datetime_module, "date"); - time_class = PyObject_GetAttrString(datetime_module, "time"); - Py_DECREF(datetime_module); - if (!datetime_class || !date_class || !time_class) - throw py::error_already_set(); + try { + // Cache datetime.datetime, datetime.date, datetime.time for isinstance + // checks in DetectParamTypes. Single import, then borrowed refs held + // for process lifetime (module-level statics, never decremented). + PyObject* datetime_module = PyImport_ImportModule("datetime"); + if (!datetime_module) throw py::error_already_set(); + datetime_class = PyObject_GetAttrString(datetime_module, "datetime"); + date_class = PyObject_GetAttrString(datetime_module, "date"); + time_class = PyObject_GetAttrString(datetime_module, "time"); + Py_DECREF(datetime_module); + if (!datetime_class || !date_class || !time_class) { + throw py::error_already_set(); + } - PyObject* decimal_module = PyImport_ImportModule("decimal"); - if (!decimal_module) throw py::error_already_set(); - decimal_class = PyObject_GetAttrString(decimal_module, "Decimal"); - Py_DECREF(decimal_module); - if (!decimal_class) throw py::error_already_set(); - - PyObject* uuid_module = PyImport_ImportModule("uuid"); - if (!uuid_module) throw py::error_already_set(); - uuid_class = PyObject_GetAttrString(uuid_module, "UUID"); - Py_DECREF(uuid_module); - if (!uuid_class) throw py::error_already_set(); - - // Pre-compute MONEY/SMALLMONEY boundary Decimals once. DetectParamTypes - // uses PyObject_RichCompareBool against these to classify Decimal params - // into MONEY vs NUMERIC — doing exact Decimal comparison (not float cast) - // avoids boundary misclassification at the edges. - PyObject* Decimal = decimal_class; - smallmoney_min = PyObject_CallFunction(Decimal, "s", "-214748.3648"); - smallmoney_max = PyObject_CallFunction(Decimal, "s", "214748.3647"); - money_min = PyObject_CallFunction(Decimal, "s", "-922337203685477.5808"); - money_max = PyObject_CallFunction(Decimal, "s", "922337203685477.5807"); - if (!smallmoney_min || !smallmoney_max || !money_min || !money_max) { - // Partial creation — clean up whichever succeeded before throwing. + PyObject* decimal_module = PyImport_ImportModule("decimal"); + if (!decimal_module) throw py::error_already_set(); + decimal_class = PyObject_GetAttrString(decimal_module, "Decimal"); + Py_DECREF(decimal_module); + if (!decimal_class) throw py::error_already_set(); + + PyObject* uuid_module = PyImport_ImportModule("uuid"); + if (!uuid_module) throw py::error_already_set(); + uuid_class = PyObject_GetAttrString(uuid_module, "UUID"); + Py_DECREF(uuid_module); + if (!uuid_class) throw py::error_already_set(); + + // Pre-compute MONEY/SMALLMONEY boundary Decimals once. DetectParamTypes + // uses PyObject_RichCompareBool against these to classify Decimal params + // into MONEY vs NUMERIC — doing exact Decimal comparison (not float cast) + // avoids boundary misclassification at the edges. + PyObject* Decimal = decimal_class; + smallmoney_min = PyObject_CallFunction(Decimal, "s", "-214748.3648"); + smallmoney_max = PyObject_CallFunction(Decimal, "s", "214748.3647"); + money_min = PyObject_CallFunction(Decimal, "s", "-922337203685477.5808"); + money_max = PyObject_CallFunction(Decimal, "s", "922337203685477.5807"); + if (!smallmoney_min || !smallmoney_max || !money_min || !money_max) { + throw py::error_already_set(); + } + + cache_initialized = true; + } catch (...) { + Py_XDECREF(datetime_class); + Py_XDECREF(date_class); + Py_XDECREF(time_class); + Py_XDECREF(decimal_class); + Py_XDECREF(uuid_class); Py_XDECREF(smallmoney_min); Py_XDECREF(smallmoney_max); Py_XDECREF(money_min); Py_XDECREF(money_max); + datetime_class = nullptr; + date_class = nullptr; + time_class = nullptr; + decimal_class = nullptr; + uuid_class = nullptr; smallmoney_min = nullptr; smallmoney_max = nullptr; money_min = nullptr; money_max = nullptr; - throw py::error_already_set(); + throw; } - - cache_initialized = true; } } @@ -570,6 +584,23 @@ SQLDescribeParamFunc SQLDescribeParam_ptr = nullptr; namespace { +// Ensures parameter bindings are always reset before local bound buffers go out +// of scope, preventing dangling APD pointers on error/exception paths. +struct ParamResetGuard { + SQLHSTMT hStmt; + bool active = false; + + explicit ParamResetGuard(SQLHSTMT stmtHandle) : hStmt(stmtHandle) {} + + ~ParamResetGuard() { + if (active && SQLFreeStmt_ptr) { + SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); + } + } + + void arm() { active = true; } +}; + const char* GetSqlCTypeAsString(const SQLSMALLINT cType) { switch (cType) { STRINGIFY_FOR_CASE(SQL_C_CHAR); @@ -929,6 +960,13 @@ std::vector DetectParamTypes(PyObject* params) { throw py::error_already_set(); } + if (!PyTuple_Check(digits_obj)) { + Py_DECREF(digits_obj); + Py_DECREF(exponent_obj); + Py_DECREF(as_tuple); + throw py::type_error("Decimal.as_tuple().digits must be a tuple"); + } + Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj); int exponent = static_cast(PyLong_AsLong(exponent_obj)); Py_DECREF(exponent_obj); @@ -1094,6 +1132,12 @@ static NumericData build_numeric_data(PyObject* decimal_param) { } Py_DECREF(exponent_obj); + if (!PyTuple_Check(digits_obj)) { + Py_DECREF(digits_obj); + Py_DECREF(as_tuple); + throw py::type_error("Decimal.as_tuple().digits must be a tuple"); + } + int num_digits = static_cast(PyTuple_GET_SIZE(digits_obj)); int precision, scale; if (exponent >= 0) { @@ -2662,6 +2706,8 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u if (!SQL_SUCCEEDED(rc)) { return rc; } + ParamResetGuard resetGuard(hStmt); + resetGuard.arm(); { // Release the GIL during the blocking SQLExecute network call. @@ -2804,11 +2850,6 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u rc, (void*)hStmt); return rc; } - - // Unbind the bound buffers for all parameters coz the buffers' memory - // will be freed when this function exits (parambuffers goes out of - // scope) - rc = SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); return rc; } } @@ -2892,6 +2933,8 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, std::vector> paramBuffers; rc = BindParameters(*statementHandle, hStmt, params, paramInfos, paramBuffers, charEncoding); if (!SQL_SUCCEEDED(rc)) return rc; + ParamResetGuard resetGuard(hStmt); + resetGuard.arm(); { py::gil_scoped_release release; @@ -3002,11 +3045,7 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc; - // Preserve the execute return code (e.g. SQL_SUCCESS_WITH_INFO) — don't - // let the SQLFreeStmt return value clobber what the caller needs to see. - SQLRETURN exec_rc = rc; - SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); - return exec_rc; + return rc; } SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list& columnwise_params, From fd06b80922a7559e347b199fcf760fda8e494f1f Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 15 Jul 2026 11:53:54 +0530 Subject: [PATCH 19/23] =?UTF-8?q?FIX:=20Revert=20RAII=20ParamResetGuard=20?= =?UTF-8?q?=E2=80=94=20it=20wiped=20ODBC=20diagnostics=20on=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParamResetGuard called SQLFreeStmt(SQL_RESET_PARAMS) in its destructor before the caller could read SQLGetDiagRec, producing empty SQLSTATEs. Restore manual SQLFreeStmt on success-only paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 31 ++++++++------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 37f2eb69..d29c2ab1 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -584,22 +584,6 @@ SQLDescribeParamFunc SQLDescribeParam_ptr = nullptr; namespace { -// Ensures parameter bindings are always reset before local bound buffers go out -// of scope, preventing dangling APD pointers on error/exception paths. -struct ParamResetGuard { - SQLHSTMT hStmt; - bool active = false; - - explicit ParamResetGuard(SQLHSTMT stmtHandle) : hStmt(stmtHandle) {} - - ~ParamResetGuard() { - if (active && SQLFreeStmt_ptr) { - SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); - } - } - - void arm() { active = true; } -}; const char* GetSqlCTypeAsString(const SQLSMALLINT cType) { switch (cType) { @@ -2706,9 +2690,6 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u if (!SQL_SUCCEEDED(rc)) { return rc; } - ParamResetGuard resetGuard(hStmt); - resetGuard.arm(); - { // Release the GIL during the blocking SQLExecute network call. py::gil_scoped_release release; @@ -2850,6 +2831,10 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u rc, (void*)hStmt); return rc; } + + // Unbind parameter buffers before they go out of scope. + // Not called on error paths — diagnostics must remain readable. + SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); return rc; } } @@ -2933,8 +2918,6 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, std::vector> paramBuffers; rc = BindParameters(*statementHandle, hStmt, params, paramInfos, paramBuffers, charEncoding); if (!SQL_SUCCEEDED(rc)) return rc; - ParamResetGuard resetGuard(hStmt); - resetGuard.arm(); { py::gil_scoped_release release; @@ -3045,7 +3028,11 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc; - return rc; + // Unbind parameter buffers before they go out of scope. + // Not called on error paths — diagnostics must remain readable. + SQLRETURN exec_rc = rc; + SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS); + return exec_rc; } SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list& columnwise_params, From 48ba475d6655bd60513a3ffcf5754d6932e870a3 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 15 Jul 2026 12:12:56 +0530 Subject: [PATCH 20/23] =?UTF-8?q?REFACTOR:=20Extract=20helpers=20=E2=80=94?= =?UTF-8?q?=20PyObjGuard,=20stream=5Fdae=5Fchunks,=20get=5Fcached=5Fclass,?= =?UTF-8?q?=20import=5Fattr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PyObjGuard: RAII cleanup for Decimal tuple extraction in DetectParamTypes and build_numeric_data (eliminates ~15 manual decref cascades) - stream_dae_chunks(): template replacing 6 identical DAE chunking loops across legacy and fast execute paths - get_cached_class(): single helper replacing 5 copy-paste type getter functions - import_attr(): consolidates import-module-getattr-decref pattern in PythonObjectCache::initialize() Net -94 lines, no functional changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 400 ++++++++++---------------- 1 file changed, 153 insertions(+), 247 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index d29c2ab1..542fbf6b 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -107,6 +107,37 @@ PyObject* get_time_class(); py::object get_time_class_obj(); } +// RAII guard for automatic PyObject cleanup on scope exit. +// Tracks a small set of PyObject* refs and decrefs them in destructor. +class PyObjGuard { + PyObject* refs[8] = {}; + int count = 0; + +public: + void track(PyObject* obj) { + for (int i = 0; i < count; ++i) { + if (refs[i] == nullptr) { + refs[i] = obj; + return; + } + } + if (count < 8) refs[count++] = obj; + } + + void release(PyObject* obj) { + for (int i = 0; i < count; ++i) { + if (refs[i] == obj) { + refs[i] = nullptr; + return; + } + } + } + + ~PyObjGuard() { + for (int i = count - 1; i >= 0; --i) Py_XDECREF(refs[i]); + } +}; + //------------------------------------------------------------------------------------------------- //------------------------------------------------------------------------------------------------- // Logging Infrastructure: @@ -129,6 +160,24 @@ static PyObject* smallmoney_min = nullptr; static PyObject* smallmoney_max = nullptr; static bool cache_initialized = false; +static PyObject* import_attr(const char* module_name, const char* attr_name) { + PyObject* mod = PyImport_ImportModule(module_name); + if (!mod) throw py::error_already_set(); + PyObject* attr = PyObject_GetAttrString(mod, attr_name); + Py_DECREF(mod); + if (!attr) throw py::error_already_set(); + return attr; +} + +static PyObject* get_cached_class(PyObject* cached, const char* module_name, const char* attr_name) { + if (cache_initialized && cached) return cached; + PyObject* mod = PyImport_ImportModule(module_name); + if (!mod) return nullptr; + PyObject* cls = PyObject_GetAttrString(mod, attr_name); + Py_DECREF(mod); + return cls; +} + // One-time cache of Python type objects and MONEY boundary constants. // Called on first execute(). Uses raw CPython API (not pybind11) because // these cached PyObject* are compared via PyObject_IsInstance in the @@ -155,17 +204,8 @@ void initialize() { throw py::error_already_set(); } - PyObject* decimal_module = PyImport_ImportModule("decimal"); - if (!decimal_module) throw py::error_already_set(); - decimal_class = PyObject_GetAttrString(decimal_module, "Decimal"); - Py_DECREF(decimal_module); - if (!decimal_class) throw py::error_already_set(); - - PyObject* uuid_module = PyImport_ImportModule("uuid"); - if (!uuid_module) throw py::error_already_set(); - uuid_class = PyObject_GetAttrString(uuid_module, "UUID"); - Py_DECREF(uuid_module); - if (!uuid_class) throw py::error_already_set(); + decimal_class = import_attr("decimal", "Decimal"); + uuid_class = import_attr("uuid", "UUID"); // Pre-compute MONEY/SMALLMONEY boundary Decimals once. DetectParamTypes // uses PyObject_RichCompareBool against these to classify Decimal params @@ -216,14 +256,7 @@ static py::object wrap_cached_or_imported(PyObject* obj) { // Returns cached type. Fallback import only for edge case where cache wasn't initialized // (e.g., called from legacy path before any fast-path execute). PyObject* get_datetime_class() { - if (cache_initialized && datetime_class) { - return datetime_class; - } - PyObject* mod = PyImport_ImportModule("datetime"); - if (!mod) return nullptr; - PyObject* cls = PyObject_GetAttrString(mod, "datetime"); - Py_DECREF(mod); - return cls; // caller must check for NULL + return get_cached_class(datetime_class, "datetime", "datetime"); } py::object get_datetime_class_obj() { @@ -233,14 +266,7 @@ py::object get_datetime_class_obj() { // Returns cached type. Fallback import only for edge case where cache wasn't initialized // (e.g., called from legacy path before any fast-path execute). PyObject* get_date_class() { - if (cache_initialized && date_class) { - return date_class; - } - PyObject* mod = PyImport_ImportModule("datetime"); - if (!mod) return nullptr; - PyObject* cls = PyObject_GetAttrString(mod, "date"); - Py_DECREF(mod); - return cls; // caller must check for NULL + return get_cached_class(date_class, "datetime", "date"); } py::object get_date_class_obj() { @@ -250,14 +276,7 @@ py::object get_date_class_obj() { // Returns cached type. Fallback import only for edge case where cache wasn't initialized // (e.g., called from legacy path before any fast-path execute). PyObject* get_time_class() { - if (cache_initialized && time_class) { - return time_class; - } - PyObject* mod = PyImport_ImportModule("datetime"); - if (!mod) return nullptr; - PyObject* cls = PyObject_GetAttrString(mod, "time"); - Py_DECREF(mod); - return cls; // caller must check for NULL + return get_cached_class(time_class, "datetime", "time"); } py::object get_time_class_obj() { @@ -267,14 +286,7 @@ py::object get_time_class_obj() { // Returns cached type. Fallback import only for edge case where cache wasn't initialized // (e.g., called from legacy path before any fast-path execute). PyObject* get_decimal_class() { - if (cache_initialized && decimal_class) { - return decimal_class; - } - PyObject* mod = PyImport_ImportModule("decimal"); - if (!mod) return nullptr; - PyObject* cls = PyObject_GetAttrString(mod, "Decimal"); - Py_DECREF(mod); - return cls; // caller must check for NULL + return get_cached_class(decimal_class, "decimal", "Decimal"); } py::object get_decimal_class_obj() { @@ -284,14 +296,7 @@ py::object get_decimal_class_obj() { // Returns cached type. Fallback import only for edge case where cache wasn't initialized // (e.g., called from legacy path before any fast-path execute). PyObject* get_uuid_class() { - if (cache_initialized && uuid_class) { - return uuid_class; - } - PyObject* mod = PyImport_ImportModule("uuid"); - if (!mod) return nullptr; - PyObject* cls = PyObject_GetAttrString(mod, "UUID"); - Py_DECREF(mod); - return cls; // caller must check for NULL + return get_cached_class(uuid_class, "uuid", "UUID"); } py::object get_uuid_class_obj() { @@ -922,43 +927,32 @@ std::vector DetectParamTypes(PyObject* params) { if (is_decimal == 1) { PyObject* as_tuple = PyObject_CallMethod(obj, "as_tuple", NULL); if (!as_tuple) throw py::error_already_set(); + PyObjGuard guard; + guard.track(as_tuple); PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); - if (!exponent_obj) { - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!exponent_obj) throw py::error_already_set(); + guard.track(exponent_obj); // NaN / Infinity / sNaN: refuse rather than silently writing 0. if (PyUnicode_Check(exponent_obj)) { - Py_DECREF(exponent_obj); - Py_DECREF(as_tuple); throw py::value_error( "Cannot bind non-finite Decimal (NaN/Infinity) as SQL NUMERIC"); } PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); - if (!digits_obj) { - Py_DECREF(exponent_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!digits_obj) throw py::error_already_set(); + guard.track(digits_obj); if (!PyTuple_Check(digits_obj)) { - Py_DECREF(digits_obj); - Py_DECREF(exponent_obj); - Py_DECREF(as_tuple); throw py::type_error("Decimal.as_tuple().digits must be a tuple"); } Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj); int exponent = static_cast(PyLong_AsLong(exponent_obj)); + guard.release(exponent_obj); Py_DECREF(exponent_obj); - if (exponent == -1 && PyErr_Occurred()) { - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set(); int precision; if (exponent >= 0) @@ -969,8 +963,6 @@ std::vector DetectParamTypes(PyObject* params) { precision = -exponent; if (precision > MAX_NUMERIC_PRECISION) { - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); throw py::value_error( "Precision of the numeric value is too high. " "The maximum precision supported by SQL Server is " + @@ -985,21 +977,13 @@ std::vector DetectParamTypes(PyObject* params) { bool in_money_range = false; int cmp_ge = PyObject_RichCompareBool(obj, PythonObjectCache::smallmoney_min, Py_GE); int cmp_le = PyObject_RichCompareBool(obj, PythonObjectCache::smallmoney_max, Py_LE); - if (cmp_ge == -1 || cmp_le == -1) { - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + 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, PythonObjectCache::money_min, Py_GE); cmp_le = PyObject_RichCompareBool(obj, PythonObjectCache::money_max, Py_LE); - if (cmp_ge == -1 || cmp_le == -1) { - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (cmp_ge == -1 || cmp_le == -1) throw py::error_already_set(); if (cmp_ge == 1 && cmp_le == 1) { in_money_range = true; } @@ -1007,16 +991,14 @@ std::vector DetectParamTypes(PyObject* params) { if (in_money_range) { PyObject* formatted = PyObject_CallMethod(obj, "__format__", "s", "f"); - if (!formatted) { - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!formatted) throw py::error_already_set(); info.paramSQLType = SQL_VARCHAR; info.paramCType = PARAM_C_TYPE_TEXT; info.columnSize = PyUnicode_GET_LENGTH(formatted); info.decimalDigits = 0; + guard.release(digits_obj); Py_DECREF(digits_obj); + guard.release(as_tuple); Py_DECREF(as_tuple); if (PyList_SetItem(params, i, formatted) != 0) { throw py::error_already_set(); @@ -1028,7 +1010,9 @@ std::vector DetectParamTypes(PyObject* params) { // object in the param list so BindParameters can extract it as NumericData. info.paramSQLType = SQL_NUMERIC; info.paramCType = SQL_C_NUMERIC; + guard.release(digits_obj); Py_DECREF(digits_obj); + guard.release(as_tuple); Py_DECREF(as_tuple); NumericData nd = build_numeric_data(obj); info.columnSize = nd.precision; @@ -1075,50 +1059,33 @@ std::vector DetectParamTypes(PyObject* params) { static NumericData build_numeric_data(PyObject* decimal_param) { PyObject* as_tuple = PyObject_CallMethod(decimal_param, "as_tuple", NULL); if (!as_tuple) throw py::error_already_set(); + PyObjGuard guard; + guard.track(as_tuple); PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); - if (!digits_obj) { - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!digits_obj) throw py::error_already_set(); + guard.track(digits_obj); PyObject* sign_obj = PyObject_GetAttrString(as_tuple, "sign"); - if (!sign_obj) { - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!sign_obj) throw py::error_already_set(); + guard.track(sign_obj); PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); - if (!exponent_obj) { - Py_DECREF(sign_obj); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!exponent_obj) throw py::error_already_set(); + guard.track(exponent_obj); int sign_val = static_cast(PyLong_AsLong(sign_obj)); + guard.release(sign_obj); Py_DECREF(sign_obj); - if (sign_val == -1 && PyErr_Occurred()) { - Py_DECREF(exponent_obj); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (sign_val == -1 && PyErr_Occurred()) throw py::error_already_set(); int exponent = 0; if (PyLong_Check(exponent_obj)) { exponent = static_cast(PyLong_AsLong(exponent_obj)); - if (exponent == -1 && PyErr_Occurred()) { - Py_DECREF(exponent_obj); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set(); } + guard.release(exponent_obj); Py_DECREF(exponent_obj); if (!PyTuple_Check(digits_obj)) { - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); throw py::type_error("Decimal.as_tuple().digits must be a tuple"); } @@ -1136,11 +1103,9 @@ static NumericData build_numeric_data(PyObject* decimal_param) { PyObject* py_ten = PyLong_FromLong(10); PyObject* int_val = PyLong_FromLong(0); + guard.track(py_ten); + guard.track(int_val); if (!py_ten || !int_val) { - Py_XDECREF(int_val); - Py_XDECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); throw py::error_already_set(); } @@ -1148,91 +1113,70 @@ static NumericData build_numeric_data(PyObject* decimal_param) { for (Py_ssize_t i = 0; i < digit_count; ++i) { PyObject* digit_obj = PyTuple_GET_ITEM(digits_obj, i); long digit = PyLong_AsLong(digit_obj); - if (digit == -1 && PyErr_Occurred()) { - Py_DECREF(int_val); - Py_DECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (digit == -1 && PyErr_Occurred()) throw py::error_already_set(); PyObject* multiplied = PyNumber_Multiply(int_val, py_ten); + guard.track(multiplied); + guard.release(int_val); Py_DECREF(int_val); - if (!multiplied) { - Py_DECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!multiplied) throw py::error_already_set(); PyObject* py_digit = PyLong_FromLong(digit); - if (!py_digit) { - Py_DECREF(multiplied); - Py_DECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!py_digit) throw py::error_already_set(); + guard.track(py_digit); int_val = PyNumber_Add(multiplied, py_digit); + guard.track(int_val); + guard.release(multiplied); Py_DECREF(multiplied); + guard.release(py_digit); Py_DECREF(py_digit); - if (!int_val) { - Py_DECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!int_val) throw py::error_already_set(); } if (exponent > 0) { PyObject* multiplier = PyLong_FromLong(1); - if (!multiplier) { - Py_DECREF(int_val); - Py_DECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!multiplier) throw py::error_already_set(); + guard.track(multiplier); for (int j = 0; j < exponent; ++j) { PyObject* next_multiplier = PyNumber_Multiply(multiplier, py_ten); + guard.track(next_multiplier); + guard.release(multiplier); Py_DECREF(multiplier); - if (!next_multiplier) { - Py_DECREF(int_val); - Py_DECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!next_multiplier) throw py::error_already_set(); multiplier = next_multiplier; } PyObject* scaled_val = PyNumber_Multiply(int_val, multiplier); + guard.track(scaled_val); + guard.release(multiplier); Py_DECREF(multiplier); + guard.release(int_val); Py_DECREF(int_val); - if (!scaled_val) { - Py_DECREF(py_ten); - Py_DECREF(digits_obj); - Py_DECREF(as_tuple); - throw py::error_already_set(); - } + if (!scaled_val) throw py::error_already_set(); int_val = scaled_val; } PyObject* abs_val = PyNumber_Absolute(int_val); + guard.track(abs_val); + guard.release(int_val); Py_DECREF(int_val); + guard.release(py_ten); Py_DECREF(py_ten); + guard.release(digits_obj); Py_DECREF(digits_obj); + guard.release(as_tuple); Py_DECREF(as_tuple); if (!abs_val) throw py::error_already_set(); PyObject* val_bytes = PyObject_CallMethod(abs_val, "to_bytes", "is", 16, "little"); + guard.track(val_bytes); + guard.release(abs_val); Py_DECREF(abs_val); if (!val_bytes) throw py::error_already_set(); char* val_buf = nullptr; Py_ssize_t val_size = 0; if (PyBytes_AsStringAndSize(val_bytes, &val_buf, &val_size) == -1) { - Py_DECREF(val_bytes); throw py::error_already_set(); } @@ -1250,11 +1194,24 @@ static NumericData build_numeric_data(PyObject* decimal_param) { std::memcpy(&nd.val[0], val_buf, copy_len); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling) #endif } + guard.release(val_bytes); Py_DECREF(val_bytes); return nd; } +template +static SQLRETURN stream_dae_chunks(const void* data, size_t total_bytes, PutDataFn put_data_fn) { + const char* bytes = static_cast(data); + for (size_t offset = 0; offset < total_bytes; offset += DAE_CHUNK_SIZE) { + size_t len = std::min(static_cast(DAE_CHUNK_SIZE), total_bytes - offset); + SQLRETURN rc = put_data_fn( + static_cast(const_cast(bytes + offset)), static_cast(len)); + if (!SQL_SUCCEEDED(rc)) return rc; + } + return SQL_SUCCESS; +} + // GH-610: Resolve SQL type for a NULL parameter using per-handle cache. // On cache miss, calls SQLDescribeParam and stores the result. static DescribedParamInfo ResolveNullParamType(SqlHandle& handle, SQLHANDLE hStmt, int paramIndex) { @@ -2731,27 +2688,13 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u if (matchedInfo->paramCType == SQL_C_WCHAR) { std::u16string utf16 = py::reinterpret_borrow(py::handle(pyObj)).cast(); - size_t totalChars = utf16.size(); - const SQLWCHAR* dataPtr = reinterpretU16stringAsSqlWChar(utf16); - size_t offset = 0; - size_t chunkChars = DAE_CHUNK_SIZE / sizeof(SQLWCHAR); - while (offset < totalChars) { - size_t len = std::min(chunkChars, totalChars - offset); - size_t lenBytes = len * sizeof(SQLWCHAR); - if (lenBytes > - static_cast(std::numeric_limits::max())) { - ThrowStdException("Chunk size exceeds maximum " - "allowed by SQLLEN"); - } - rc = putData((SQLPOINTER)(dataPtr + offset), - static_cast(lenBytes)); - if (!SQL_SUCCEEDED(rc)) { - LOG("SQLExecute: SQLPutData failed for " - "SQL_C_WCHAR chunk - offset=%zu", - offset, totalChars, lenBytes, rc); - return rc; - } - offset += len; + rc = stream_dae_chunks( + reinterpretU16stringAsSqlWChar(utf16), + utf16.size() * sizeof(SQLWCHAR), + putData); + if (!SQL_SUCCEEDED(rc)) { + LOG("SQLExecute: SQLPutData failed for SQL_C_WCHAR DAE streaming"); + return rc; } } else if (matchedInfo->paramCType == SQL_C_CHAR) { // Encode the string using the specified encoding @@ -2768,21 +2711,10 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u throw; } - size_t totalBytes = encodedStr.size(); - const char* dataPtr = encodedStr.data(); - size_t offset = 0; - size_t chunkBytes = DAE_CHUNK_SIZE; - while (offset < totalBytes) { - size_t len = std::min(chunkBytes, totalBytes - offset); - - rc = putData((SQLPOINTER)(dataPtr + offset), static_cast(len)); - if (!SQL_SUCCEEDED(rc)) { - LOG("SQLExecute: SQLPutData failed for " - "SQL_C_CHAR chunk - offset=%zu", - offset, totalBytes, len, rc); - return rc; - } - offset += len; + rc = stream_dae_chunks(encodedStr.data(), encodedStr.size(), putData); + if (!SQL_SUCCEEDED(rc)) { + LOG("SQLExecute: SQLPutData failed for SQL_C_CHAR DAE streaming"); + return rc; } } else { ThrowStdException("Unsupported C type for str in DAE"); @@ -2802,16 +2734,11 @@ SQLRETURN SQLExecuteLegacy_wrap(const SqlHandlePtr statementHandle, const std::u dataPtr = bytesStorage.data(); totalBytes = bytesStorage.size(); } - const size_t chunkSize = DAE_CHUNK_SIZE; - for (size_t offset = 0; offset < totalBytes; offset += chunkSize) { - size_t len = std::min(chunkSize, totalBytes - offset); - rc = putData((SQLPOINTER)(dataPtr + offset), static_cast(len)); - if (!SQL_SUCCEEDED(rc)) { - LOG("SQLExecute: SQLPutData failed for " - "binary/bytes chunk - offset=%zu", - offset, totalBytes, len, rc); - return rc; - } + + rc = stream_dae_chunks(dataPtr, totalBytes, putData); + if (!SQL_SUCCEEDED(rc)) { + LOG("SQLExecute: SQLPutData failed for binary/bytes DAE streaming"); + return rc; } } else { ThrowStdException("DAE only supported for str or bytes"); @@ -2930,6 +2857,10 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, // GIL is released around each ODBC call to match slow-path concurrency. if (rc == SQL_NEED_DATA) { SQLPOINTER paramToken = nullptr; + auto putData = [&](SQLPOINTER data, SQLLEN len) { + py::gil_scoped_release release; + return SQLPutData_ptr(hStmt, data, len); + }; while (true) { { py::gil_scoped_release release; @@ -2958,35 +2889,18 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, if (matchedInfo->paramCType == SQL_C_WCHAR) { std::u16string u16 = py::reinterpret_borrow(py::handle(pyObj)).cast(); - const SQLWCHAR* dataPtr = reinterpretU16stringAsSqlWChar(u16); - size_t totalChars = u16.size(); - size_t chunkChars = DAE_CHUNK_SIZE / sizeof(SQLWCHAR); - for (size_t offset = 0; offset < totalChars; offset += chunkChars) { - size_t len = std::min(chunkChars, totalChars - offset); - { - py::gil_scoped_release release; - rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), - static_cast(len * sizeof(SQLWCHAR))); - } - if (!SQL_SUCCEEDED(rc)) return rc; - } + rc = stream_dae_chunks( + reinterpretU16stringAsSqlWChar(u16), + u16.size() * sizeof(SQLWCHAR), + putData); + if (!SQL_SUCCEEDED(rc)) return rc; } else if (matchedInfo->paramCType == SQL_C_CHAR) { std::string encodedStr; py::object encoded = py::reinterpret_borrow(py::handle(pyObj)) .attr("encode")(charEncoding, "strict"); encodedStr = encoded.cast(); - const char* dataPtr = encodedStr.data(); - size_t totalBytes = encodedStr.size(); - for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { - size_t len = std::min(static_cast(DAE_CHUNK_SIZE), - totalBytes - offset); - { - py::gil_scoped_release release; - rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), - static_cast(len)); - } - if (!SQL_SUCCEEDED(rc)) return rc; - } + rc = stream_dae_chunks(encodedStr.data(), encodedStr.size(), putData); + if (!SQL_SUCCEEDED(rc)) return rc; } else { ThrowStdException("SQLExecuteFast: unsupported C type for str in DAE"); } @@ -3009,16 +2923,8 @@ SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle, totalBytes = bytesStorage.size(); } - for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) { - size_t len = std::min(static_cast(DAE_CHUNK_SIZE), - totalBytes - offset); - { - py::gil_scoped_release release; - rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset), - static_cast(len)); - } - if (!SQL_SUCCEEDED(rc)) return rc; - } + rc = stream_dae_chunks(dataPtr, totalBytes, putData); + if (!SQL_SUCCEEDED(rc)) return rc; } else { ThrowStdException("SQLExecuteFast: DAE only supported for str or bytes"); } From 114d76b3dd737dd044908590b4ab744d015a3206 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 15 Jul 2026 12:16:59 +0530 Subject: [PATCH 21/23] FIX: Suppress DevSkim DS121708 for bounded memcpy on non-MSVC Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 542fbf6b..44083c59 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -1191,7 +1191,7 @@ static NumericData build_numeric_data(PyObject* decimal_param) { memcpy_s(&nd.val[0], SQL_MAX_NUMERIC_LEN, val_buf, copy_len); #else // copy_len is bounded to SQL_MAX_NUMERIC_LEN above — safe by construction - std::memcpy(&nd.val[0], val_buf, copy_len); // NOLINT(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling) + std::memcpy(&nd.val[0], val_buf, copy_len); // DevSkim: ignore DS121708 #endif } guard.release(val_bytes); From 3499dbdeb31ffe433f377c25298329d3876b9283 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 15 Jul 2026 12:40:55 +0530 Subject: [PATCH 22/23] DOC: Add algorithm comments to build_numeric_data and Decimal detection Explain the SQL_NUMERIC_STRUCT conversion algorithm step-by-step, precision/scale computation logic, MONEY range check rationale, and one-liners on helper utilities (PyObjGuard, stream_dae_chunks, get_cached_class). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 44083c59..dc6a3869 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -124,6 +124,7 @@ class PyObjGuard { if (count < 8) refs[count++] = obj; } + // release() hands ownership back to a caller that decrefs immediately, avoiding double-decref in ~PyObjGuard(). void release(PyObject* obj) { for (int i = 0; i < count; ++i) { if (refs[i] == obj) { @@ -169,6 +170,7 @@ static PyObject* import_attr(const char* module_name, const char* attr_name) { return attr; } +// Fall back to import here because legacy paths can reach these lookups before initialize_cache() has populated globals. static PyObject* get_cached_class(PyObject* cached, const char* module_name, const char* attr_name) { if (cache_initialized && cached) return cached; PyObject* mod = PyImport_ImportModule(module_name); @@ -955,6 +957,9 @@ std::vector DetectParamTypes(PyObject* params) { if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set(); int precision; + // Precision is total base-10 digits after applying Decimal's exponent: positive exponents + // add trailing zeros, in-range negative exponents keep the original digit count, and larger + // negative exponents force leading fractional zeros such as Decimal("0.001") -> precision 3. if (exponent >= 0) precision = static_cast(num_digits) + exponent; else if ((-exponent) <= num_digits) @@ -970,6 +975,8 @@ 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. @@ -1057,11 +1064,14 @@ std::vector DetectParamTypes(PyObject* params) { // Takes raw PyObject* (must be a Decimal instance). Returns NumericData directly — // the caller stores it as a Python capsule via PyList_SetItem for the binder to consume. static NumericData build_numeric_data(PyObject* decimal_param) { + // Decimal.as_tuple() exposes the canonical pieces we need: sign, coefficient digits, and exponent. PyObject* as_tuple = PyObject_CallMethod(decimal_param, "as_tuple", NULL); if (!as_tuple) throw py::error_already_set(); PyObjGuard guard; guard.track(as_tuple); + // Step 1-4: unpack Decimal(sign, digits, exponent) and validate that digits is the tuple form + // promised by Decimal.as_tuple(); the remaining logic works purely from these normalized parts. PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); if (!digits_obj) throw py::error_already_set(); guard.track(digits_obj); @@ -1089,6 +1099,9 @@ static NumericData build_numeric_data(PyObject* decimal_param) { throw py::type_error("Decimal.as_tuple().digits must be a tuple"); } + // Step 5: SQL Server precision counts all stored decimal digits, while scale is just the fractional + // digits. A positive exponent means trailing zeros move into the integer part; a negative exponent + // means scale = -exponent and precision must still cover leading fractional zeros such as 0.001. int num_digits = static_cast(PyTuple_GET_SIZE(digits_obj)); int precision, scale; if (exponent >= 0) { @@ -1101,6 +1114,8 @@ static NumericData build_numeric_data(PyObject* decimal_param) { precision = std::max(1, std::min(precision, MAX_NUMERIC_PRECISION)); scale = std::min(scale, precision); + // Step 6: build the mantissa with Python bigint math so we preserve every Decimal digit; a fixed + // C++ integer would overflow long before SQL Server's 38-digit NUMERIC limit. PyObject* py_ten = PyLong_FromLong(10); PyObject* int_val = PyLong_FromLong(0); guard.track(py_ten); @@ -1134,6 +1149,8 @@ static NumericData build_numeric_data(PyObject* decimal_param) { if (!int_val) throw py::error_already_set(); } + // Step 7: a positive Decimal exponent means the tuple digits omit trailing zeros, so Decimal("123e2") + // must become integer mantissa 12300 before packing. if (exponent > 0) { PyObject* multiplier = PyLong_FromLong(1); if (!multiplier) throw py::error_already_set(); @@ -1156,6 +1173,7 @@ static NumericData build_numeric_data(PyObject* decimal_param) { int_val = scaled_val; } + // Step 8: SQL_NUMERIC_STRUCT stores magnitude and sign separately, so encode only the absolute value. PyObject* abs_val = PyNumber_Absolute(int_val); guard.track(abs_val); guard.release(int_val); @@ -1168,6 +1186,8 @@ static NumericData build_numeric_data(PyObject* decimal_param) { Py_DECREF(as_tuple); if (!abs_val) throw py::error_already_set(); + // Step 9: SQL_NUMERIC_STRUCT::val is a 16-byte little-endian unsigned integer buffer, so ask + // Python's bigint to serialize directly into that wire format instead of reimplementing base conversion. PyObject* val_bytes = PyObject_CallMethod(abs_val, "to_bytes", "is", 16, "little"); guard.track(val_bytes); guard.release(abs_val); @@ -1180,6 +1200,8 @@ static NumericData build_numeric_data(PyObject* decimal_param) { throw py::error_already_set(); } + // Step 10: pack precision/scale plus the 16-byte little-endian magnitude. SQL uses sign=1 for + // positive and sign=0 for negative, which is the inverse of Decimal.as_tuple().sign. NumericData nd; nd.precision = static_cast(precision); nd.scale = static_cast(scale); @@ -1201,6 +1223,7 @@ static NumericData build_numeric_data(PyObject* decimal_param) { } template +// The callable hides whether the caller wraps SQLPutData with GIL management; chunk sizing stays shared. static SQLRETURN stream_dae_chunks(const void* data, size_t total_bytes, PutDataFn put_data_fn) { const char* bytes = static_cast(data); for (size_t offset = 0; offset < total_bytes; offset += DAE_CHUNK_SIZE) { From d164a540cb51d119107384ca03a9a7c781518622 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 15 Jul 2026 13:31:28 +0530 Subject: [PATCH 23/23] REFACTOR: Replace PyObjGuard with PyPtr (Pattern 0 from perf proposal) Introduce py_ref.hpp: PyPtr = std::unique_ptr with adopt() and incref_borrow() helpers. Zero runtime overhead via empty-base optimisation. Replaces the bespoke PyObjGuard (fixed 8-slot array, manual track/release) with standard C++ RAII throughout DetectParamTypes and build_numeric_data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 180 ++++++++------------------ mssql_python/pybind/py_ref.hpp | 26 ++++ 2 files changed, 77 insertions(+), 129 deletions(-) create mode 100644 mssql_python/pybind/py_ref.hpp diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index dc6a3869..466cf3bf 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -8,8 +8,13 @@ #include "connection/connection.h" #include "connection/connection_pool.h" #include "logger_bridge.hpp" +#include "py_ref.hpp" #include "utf_utils.h" +using pyref::PyPtr; +using pyref::adopt; +using pyref::incref_borrow; + #include // std::min #include @@ -107,38 +112,6 @@ PyObject* get_time_class(); py::object get_time_class_obj(); } -// RAII guard for automatic PyObject cleanup on scope exit. -// Tracks a small set of PyObject* refs and decrefs them in destructor. -class PyObjGuard { - PyObject* refs[8] = {}; - int count = 0; - -public: - void track(PyObject* obj) { - for (int i = 0; i < count; ++i) { - if (refs[i] == nullptr) { - refs[i] = obj; - return; - } - } - if (count < 8) refs[count++] = obj; - } - - // release() hands ownership back to a caller that decrefs immediately, avoiding double-decref in ~PyObjGuard(). - void release(PyObject* obj) { - for (int i = 0; i < count; ++i) { - if (refs[i] == obj) { - refs[i] = nullptr; - return; - } - } - } - - ~PyObjGuard() { - for (int i = count - 1; i >= 0; --i) Py_XDECREF(refs[i]); - } -}; - //------------------------------------------------------------------------------------------------- //------------------------------------------------------------------------------------------------- // Logging Infrastructure: @@ -927,33 +900,27 @@ std::vector DetectParamTypes(PyObject* params) { int is_decimal = PyObject_IsInstance(obj, decimal_type); if (is_decimal == -1) throw py::error_already_set(); if (is_decimal == 1) { - PyObject* as_tuple = PyObject_CallMethod(obj, "as_tuple", NULL); - if (!as_tuple) throw py::error_already_set(); - PyObjGuard guard; - guard.track(as_tuple); + PyPtr as_tuple_ptr = adopt(PyObject_CallMethod(obj, "as_tuple", NULL)); + if (!as_tuple_ptr) throw py::error_already_set(); - PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); + PyPtr exponent_obj = adopt(PyObject_GetAttrString(as_tuple_ptr.get(), "exponent")); if (!exponent_obj) throw py::error_already_set(); - guard.track(exponent_obj); // NaN / Infinity / sNaN: refuse rather than silently writing 0. - if (PyUnicode_Check(exponent_obj)) { + if (PyUnicode_Check(exponent_obj.get())) { throw py::value_error( "Cannot bind non-finite Decimal (NaN/Infinity) as SQL NUMERIC"); } - PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); + PyPtr digits_obj = adopt(PyObject_GetAttrString(as_tuple_ptr.get(), "digits")); if (!digits_obj) throw py::error_already_set(); - guard.track(digits_obj); - if (!PyTuple_Check(digits_obj)) { + if (!PyTuple_Check(digits_obj.get())) { throw py::type_error("Decimal.as_tuple().digits must be a tuple"); } - Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj); - int exponent = static_cast(PyLong_AsLong(exponent_obj)); - guard.release(exponent_obj); - Py_DECREF(exponent_obj); + Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj.get()); + int exponent = static_cast(PyLong_AsLong(exponent_obj.get())); if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set(); int precision; @@ -997,17 +964,15 @@ std::vector DetectParamTypes(PyObject* params) { } if (in_money_range) { - PyObject* formatted = PyObject_CallMethod(obj, "__format__", "s", "f"); + PyPtr formatted = adopt(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); + info.columnSize = PyUnicode_GET_LENGTH(formatted.get()); info.decimalDigits = 0; - guard.release(digits_obj); - Py_DECREF(digits_obj); - guard.release(as_tuple); - Py_DECREF(as_tuple); - if (PyList_SetItem(params, i, formatted) != 0) { + PyObject* raw = formatted.release(); + if (PyList_SetItem(params, i, raw) != 0) { + Py_DECREF(raw); throw py::error_already_set(); } continue; @@ -1017,10 +982,6 @@ std::vector DetectParamTypes(PyObject* params) { // object in the param list so BindParameters can extract it as NumericData. info.paramSQLType = SQL_NUMERIC; info.paramCType = SQL_C_NUMERIC; - guard.release(digits_obj); - Py_DECREF(digits_obj); - guard.release(as_tuple); - Py_DECREF(as_tuple); NumericData nd = build_numeric_data(obj); info.columnSize = nd.precision; info.decimalDigits = nd.scale; @@ -1065,44 +1026,37 @@ std::vector DetectParamTypes(PyObject* params) { // the caller stores it as a Python capsule via PyList_SetItem for the binder to consume. static NumericData build_numeric_data(PyObject* decimal_param) { // Decimal.as_tuple() exposes the canonical pieces we need: sign, coefficient digits, and exponent. - PyObject* as_tuple = PyObject_CallMethod(decimal_param, "as_tuple", NULL); + PyPtr as_tuple = adopt(PyObject_CallMethod(decimal_param, "as_tuple", NULL)); if (!as_tuple) throw py::error_already_set(); - PyObjGuard guard; - guard.track(as_tuple); // Step 1-4: unpack Decimal(sign, digits, exponent) and validate that digits is the tuple form // promised by Decimal.as_tuple(); the remaining logic works purely from these normalized parts. - PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits"); - if (!digits_obj) throw py::error_already_set(); - guard.track(digits_obj); - PyObject* sign_obj = PyObject_GetAttrString(as_tuple, "sign"); + PyPtr digits = adopt(PyObject_GetAttrString(as_tuple.get(), "digits")); + if (!digits) throw py::error_already_set(); + PyPtr sign_obj = adopt(PyObject_GetAttrString(as_tuple.get(), "sign")); if (!sign_obj) throw py::error_already_set(); - guard.track(sign_obj); - PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent"); + PyPtr exponent_obj = adopt(PyObject_GetAttrString(as_tuple.get(), "exponent")); if (!exponent_obj) throw py::error_already_set(); - guard.track(exponent_obj); - int sign_val = static_cast(PyLong_AsLong(sign_obj)); - guard.release(sign_obj); - Py_DECREF(sign_obj); + int sign_val = static_cast(PyLong_AsLong(sign_obj.get())); + sign_obj.reset(); if (sign_val == -1 && PyErr_Occurred()) throw py::error_already_set(); int exponent = 0; - if (PyLong_Check(exponent_obj)) { - exponent = static_cast(PyLong_AsLong(exponent_obj)); + if (PyLong_Check(exponent_obj.get())) { + exponent = static_cast(PyLong_AsLong(exponent_obj.get())); if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set(); } - guard.release(exponent_obj); - Py_DECREF(exponent_obj); + exponent_obj.reset(); - if (!PyTuple_Check(digits_obj)) { + if (!PyTuple_Check(digits.get())) { throw py::type_error("Decimal.as_tuple().digits must be a tuple"); } // Step 5: SQL Server precision counts all stored decimal digits, while scale is just the fractional // digits. A positive exponent means trailing zeros move into the integer part; a negative exponent // means scale = -exponent and precision must still cover leading fractional zeros such as 0.001. - int num_digits = static_cast(PyTuple_GET_SIZE(digits_obj)); + int num_digits = static_cast(PyTuple_GET_SIZE(digits.get())); int precision, scale; if (exponent >= 0) { precision = num_digits + exponent; @@ -1116,87 +1070,58 @@ static NumericData build_numeric_data(PyObject* decimal_param) { // Step 6: build the mantissa with Python bigint math so we preserve every Decimal digit; a fixed // C++ integer would overflow long before SQL Server's 38-digit NUMERIC limit. - PyObject* py_ten = PyLong_FromLong(10); - PyObject* int_val = PyLong_FromLong(0); - guard.track(py_ten); - guard.track(int_val); + PyPtr py_ten = adopt(PyLong_FromLong(10)); + PyPtr int_val = adopt(PyLong_FromLong(0)); if (!py_ten || !int_val) { throw py::error_already_set(); } - const Py_ssize_t digit_count = PyTuple_GET_SIZE(digits_obj); + const Py_ssize_t digit_count = PyTuple_GET_SIZE(digits.get()); for (Py_ssize_t i = 0; i < digit_count; ++i) { - PyObject* digit_obj = PyTuple_GET_ITEM(digits_obj, i); + PyObject* digit_obj = PyTuple_GET_ITEM(digits.get(), i); long digit = PyLong_AsLong(digit_obj); if (digit == -1 && PyErr_Occurred()) throw py::error_already_set(); - PyObject* multiplied = PyNumber_Multiply(int_val, py_ten); - guard.track(multiplied); - guard.release(int_val); - Py_DECREF(int_val); + PyPtr multiplied = adopt(PyNumber_Multiply(int_val.get(), py_ten.get())); if (!multiplied) throw py::error_already_set(); - PyObject* py_digit = PyLong_FromLong(digit); + PyPtr py_digit = adopt(PyLong_FromLong(digit)); if (!py_digit) throw py::error_already_set(); - guard.track(py_digit); - - int_val = PyNumber_Add(multiplied, py_digit); - guard.track(int_val); - guard.release(multiplied); - Py_DECREF(multiplied); - guard.release(py_digit); - Py_DECREF(py_digit); + + int_val = adopt(PyNumber_Add(multiplied.get(), py_digit.get())); if (!int_val) throw py::error_already_set(); } // Step 7: a positive Decimal exponent means the tuple digits omit trailing zeros, so Decimal("123e2") // must become integer mantissa 12300 before packing. if (exponent > 0) { - PyObject* multiplier = PyLong_FromLong(1); + PyPtr multiplier = adopt(PyLong_FromLong(1)); if (!multiplier) throw py::error_already_set(); - guard.track(multiplier); for (int j = 0; j < exponent; ++j) { - PyObject* next_multiplier = PyNumber_Multiply(multiplier, py_ten); - guard.track(next_multiplier); - guard.release(multiplier); - Py_DECREF(multiplier); - if (!next_multiplier) throw py::error_already_set(); - multiplier = next_multiplier; + multiplier = adopt(PyNumber_Multiply(multiplier.get(), py_ten.get())); + if (!multiplier) throw py::error_already_set(); } - PyObject* scaled_val = PyNumber_Multiply(int_val, multiplier); - guard.track(scaled_val); - guard.release(multiplier); - Py_DECREF(multiplier); - guard.release(int_val); - Py_DECREF(int_val); - if (!scaled_val) throw py::error_already_set(); - int_val = scaled_val; + int_val = adopt(PyNumber_Multiply(int_val.get(), multiplier.get())); + if (!int_val) throw py::error_already_set(); } // Step 8: SQL_NUMERIC_STRUCT stores magnitude and sign separately, so encode only the absolute value. - PyObject* abs_val = PyNumber_Absolute(int_val); - guard.track(abs_val); - guard.release(int_val); - Py_DECREF(int_val); - guard.release(py_ten); - Py_DECREF(py_ten); - guard.release(digits_obj); - Py_DECREF(digits_obj); - guard.release(as_tuple); - Py_DECREF(as_tuple); + PyPtr abs_val = adopt(PyNumber_Absolute(int_val.get())); + int_val.reset(); + py_ten.reset(); + digits.reset(); + as_tuple.reset(); if (!abs_val) throw py::error_already_set(); // Step 9: SQL_NUMERIC_STRUCT::val is a 16-byte little-endian unsigned integer buffer, so ask // Python's bigint to serialize directly into that wire format instead of reimplementing base conversion. - PyObject* val_bytes = PyObject_CallMethod(abs_val, "to_bytes", "is", 16, "little"); - guard.track(val_bytes); - guard.release(abs_val); - Py_DECREF(abs_val); + PyPtr val_bytes = adopt(PyObject_CallMethod(abs_val.get(), "to_bytes", "is", 16, "little")); + abs_val.reset(); if (!val_bytes) throw py::error_already_set(); char* val_buf = nullptr; Py_ssize_t val_size = 0; - if (PyBytes_AsStringAndSize(val_bytes, &val_buf, &val_size) == -1) { + if (PyBytes_AsStringAndSize(val_bytes.get(), &val_buf, &val_size) == -1) { throw py::error_already_set(); } @@ -1216,9 +1141,6 @@ static NumericData build_numeric_data(PyObject* decimal_param) { std::memcpy(&nd.val[0], val_buf, copy_len); // DevSkim: ignore DS121708 #endif } - guard.release(val_bytes); - Py_DECREF(val_bytes); - return nd; } diff --git a/mssql_python/pybind/py_ref.hpp b/mssql_python/pybind/py_ref.hpp new file mode 100644 index 00000000..852221da --- /dev/null +++ b/mssql_python/pybind/py_ref.hpp @@ -0,0 +1,26 @@ +// py_ref.hpp — RAII wrapper for CPython refcount management (Pattern 0). +// +// PyPtr = std::unique_ptr +// +// Zero runtime overhead (stateless functor → empty-base optimisation). +// Use adopt() for new references, incref_borrow() for borrowed ones. + +#pragma once +#include +#include + +namespace pyref { + +struct PyDecRefDeleter { + void operator()(PyObject* p) const noexcept { Py_XDECREF(p); } +}; + +using PyPtr = std::unique_ptr; + +// Wrap a new reference (already +1 from the API that returned it). +inline PyPtr adopt(PyObject* p) noexcept { return PyPtr{p}; } + +// Extend a borrowed reference's lifetime past its borrower. +inline PyPtr incref_borrow(PyObject* p) noexcept { Py_XINCREF(p); return PyPtr{p}; } + +} // namespace pyref