PERF: Native C++ parameter detection and execute pipeline#549
Open
bewithgaurav wants to merge 28 commits into
Open
PERF: Native C++ parameter detection and execute pipeline#549bewithgaurav wants to merge 28 commits into
bewithgaurav wants to merge 28 commits into
Conversation
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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- 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
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 144-157 144 }
145
146 // Fall back to import here because legacy paths can reach these lookups before initialize_cache() has populated globals.
147 static PyObject* get_cached_class(PyObject* cached, const char* module_name, const char* attr_name) {
! 148 if (cache_initialized && cached) return cached;
! 149 PyObject* mod = PyImport_ImportModule(module_name);
! 150 if (!mod) return nullptr;
! 151 PyObject* cls = PyObject_GetAttrString(mod, attr_name);
! 152 Py_DECREF(mod);
! 153 return cls;
154 }
155
156 // One-time cache of Python type objects and MONEY boundary constants.
157 // Called on first execute(). Uses raw CPython API (not pybind11) becauseLines 160-169 160 // unnecessary ref-count traffic on every parameter.
161 void initialize() {
162 if (!cache_initialized) {
163 PyDateTime_IMPORT;
! 164 if (PyDateTimeAPI == nullptr) {
! 165 throw py::error_already_set();
166 }
167
168 try {
169 // Cache datetime.datetime, datetime.date, datetime.time for isinstanceLines 174-183 174 datetime_class = PyObject_GetAttrString(datetime_module, "datetime");
175 date_class = PyObject_GetAttrString(datetime_module, "date");
176 time_class = PyObject_GetAttrString(datetime_module, "time");
177 Py_DECREF(datetime_module);
! 178 if (!datetime_class || !date_class || !time_class) {
! 179 throw py::error_already_set();
180 }
181
182 decimal_class = import_attr("decimal", "Decimal");
183 uuid_class = import_attr("uuid", "UUID");Lines 190-222 190 smallmoney_min = PyObject_CallFunction(Decimal, "s", "-214748.3648");
191 smallmoney_max = PyObject_CallFunction(Decimal, "s", "214748.3647");
192 money_min = PyObject_CallFunction(Decimal, "s", "-922337203685477.5808");
193 money_max = PyObject_CallFunction(Decimal, "s", "922337203685477.5807");
! 194 if (!smallmoney_min || !smallmoney_max || !money_min || !money_max) {
! 195 throw py::error_already_set();
196 }
197
198 cache_initialized = true;
! 199 } catch (...) {
! 200 Py_XDECREF(datetime_class);
! 201 Py_XDECREF(date_class);
! 202 Py_XDECREF(time_class);
! 203 Py_XDECREF(decimal_class);
! 204 Py_XDECREF(uuid_class);
! 205 Py_XDECREF(smallmoney_min);
! 206 Py_XDECREF(smallmoney_max);
! 207 Py_XDECREF(money_min);
! 208 Py_XDECREF(money_max);
! 209 datetime_class = nullptr;
! 210 date_class = nullptr;
! 211 time_class = nullptr;
! 212 decimal_class = nullptr;
! 213 uuid_class = nullptr;
! 214 smallmoney_min = nullptr;
! 215 smallmoney_max = nullptr;
! 216 money_min = nullptr;
! 217 money_max = nullptr;
! 218 throw;
219 }
220 }
221 }Lines 316-362 316 Py_XINCREF(dataPtr);
317 }
318 // Copy/move assignment and move constructor below are required for
319 // std::vector<ParamInfo> resize/reallocation and pybind11's type_caster.
! 320 // They manually manage dataPtr refcounts since ParamInfo owns a strong ref.
! 321 ParamInfo& operator=(const ParamInfo& other) {
! 322 if (this != &other) {
! 323 Py_XDECREF(dataPtr);
! 324 inputOutputType = other.inputOutputType;
! 325 paramCType = other.paramCType;
! 326 paramSQLType = other.paramSQLType;
! 327 columnSize = other.columnSize;
! 328 decimalDigits = other.decimalDigits;
! 329 strLenOrInd = other.strLenOrInd;
! 330 isDAE = other.isDAE;
! 331 dataPtr = other.dataPtr;
! 332 utf16Len = other.utf16Len;
! 333 Py_XINCREF(dataPtr);
! 334 }
! 335 return *this;
336 }
! 337 ParamInfo(ParamInfo&& other) noexcept
! 338 : inputOutputType(other.inputOutputType), paramCType(other.paramCType),
! 339 paramSQLType(other.paramSQLType), columnSize(other.columnSize),
! 340 decimalDigits(other.decimalDigits), strLenOrInd(other.strLenOrInd),
! 341 isDAE(other.isDAE), dataPtr(other.dataPtr), utf16Len(other.utf16Len) {
! 342 other.dataPtr = nullptr;
! 343 }
! 344 ParamInfo& operator=(ParamInfo&& other) noexcept {
! 345 if (this != &other) {
! 346 Py_XDECREF(dataPtr);
! 347 inputOutputType = other.inputOutputType;
! 348 paramCType = other.paramCType;
! 349 paramSQLType = other.paramSQLType;
! 350 columnSize = other.columnSize;
! 351 decimalDigits = other.decimalDigits;
! 352 strLenOrInd = other.strLenOrInd;
! 353 isDAE = other.isDAE;
! 354 dataPtr = other.dataPtr;
! 355 utf16Len = other.utf16Len;
! 356 other.dataPtr = nullptr;
! 357 }
! 358 return *this;
359 }
360 };
361 #ifdef __GNUC__
362 #pragma GCC diagnostic popLines 563-571 563 SQLDescribeParamFunc SQLDescribeParam_ptr = nullptr;
564
565 namespace {
566
! 567
568 const char* GetSqlCTypeAsString(const SQLSMALLINT cType) {
569 switch (cType) {
570 STRINGIFY_FOR_CASE(SQL_C_CHAR);
571 STRINGIFY_FOR_CASE(SQL_C_WCHAR);Lines 889-898 889 Py_ssize_t time_len = PyUnicode_GET_LENGTH(time_str);
890 info.columnSize = std::max<SQLULEN>(info.columnSize, time_len);
891 // PyList_SetItem (lowercase) decrefs the old slot before stealing the new
892 // reference; safe here because cursor.py already passed a fresh list copy.
! 893 if (PyList_SetItem(params, i, time_str) != 0) {
! 894 throw py::error_already_set();
895 }
896 continue;
897 }Lines 914-923 914
915 PyPtr digits_obj = adopt(PyObject_GetAttrString(as_tuple_ptr.get(), "digits"));
916 if (!digits_obj) throw py::error_already_set();
917
! 918 if (!PyTuple_Check(digits_obj.get())) {
! 919 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
920 }
921
922 Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj.get());
923 int exponent = static_cast<int>(PyLong_AsLong(exponent_obj.get()));Lines 970-980 970 info.paramCType = PARAM_C_TYPE_TEXT;
971 info.columnSize = PyUnicode_GET_LENGTH(formatted.get());
972 info.decimalDigits = 0;
973 PyObject* raw = formatted.release();
! 974 if (PyList_SetItem(params, i, raw) != 0) {
! 975 Py_DECREF(raw);
! 976 throw py::error_already_set();
977 }
978 continue;
979 }Lines 987-997 987 info.decimalDigits = nd.scale;
988 // Store NumericData as a Python object in the param list for the binder.
989 py::object numeric_obj = py::cast(nd);
990 PyObject* raw = numeric_obj.release().ptr();
! 991 if (PyList_SetItem(params, i, raw) != 0) {
! 992 Py_DECREF(raw);
! 993 throw py::error_already_set();
994 }
995 continue;
996 }Lines 1004-1014 1004 info.paramSQLType = SQL_GUID;
1005 info.paramCType = SQL_C_GUID;
1006 info.columnSize = 16;
1007 info.decimalDigits = 0;
! 1008 if (PyList_SetItem(params, i, bytes_le) != 0) {
! 1009 Py_DECREF(bytes_le);
! 1010 throw py::error_already_set();
1011 }
1012 continue;
1013 }Lines 1048-1057 1048 if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set();
1049 }
1050 exponent_obj.reset();
1051
! 1052 if (!PyTuple_Check(digits.get())) {
! 1053 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
1054 }
1055
1056 // Step 5: SQL Server precision counts all stored decimal digits, while scale is just the fractional
1057 // digits. A positive exponent means trailing zeros move into the integer part; a negative exponentLines 1071-1080 1071 // Step 6: build the mantissa with Python bigint math so we preserve every Decimal digit; a fixed
1072 // C++ integer would overflow long before SQL Server's 38-digit NUMERIC limit.
1073 PyPtr py_ten = adopt(PyLong_FromLong(10));
1074 PyPtr int_val = adopt(PyLong_FromLong(0));
! 1075 if (!py_ten || !int_val) {
! 1076 throw py::error_already_set();
1077 }
1078
1079 const Py_ssize_t digit_count = PyTuple_GET_SIZE(digits.get());
1080 for (Py_ssize_t i = 0; i < digit_count; ++i) {Lines 1120-1129 1120 if (!val_bytes) throw py::error_already_set();
1121
1122 char* val_buf = nullptr;
1123 Py_ssize_t val_size = 0;
! 1124 if (PyBytes_AsStringAndSize(val_bytes.get(), &val_buf, &val_size) == -1) {
! 1125 throw py::error_already_set();
1126 }
1127
1128 // Step 10: pack precision/scale plus the 16-byte little-endian magnitude. SQL uses sign=1 for
1129 // positive and sign=0 for negative, which is the inverse of Decimal.as_tuple().sign.Lines 1399-1408 1399 dataPtr = sqlwcharBuffer->data();
1400 bufferLength = sqlwcharBuffer->size() * sizeof(SQLWCHAR);
1401 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
1402 // Use explicit byte length instead of SQL_NTS so embedded NUL chars
! 1403 // aren't treated as string terminators.
! 1404 *strLenOrIndPtr = static_cast<SQLLEN>(sqlwcharBuffer->size() * sizeof(SQLWCHAR));
1405 }
1406 break;
1407 }
1408 case SQL_C_BIT: {Lines 1541-1549 1541 dataPtr = static_cast<void*>(sqlTimePtr);
1542 break;
1543 }
1544 case SQL_C_SS_TIMESTAMPOFFSET: {
! 1545 py::object datetimeType = PythonObjectCache::get_datetime_class_obj();
1546 if (!py::isinstance(param, datetimeType)) {
1547 ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
1548 }
1549 // Checking if the object has a timezoneLines 2637-2646 2637 reinterpretU16stringAsSqlWChar(utf16),
2638 utf16.size() * sizeof(SQLWCHAR),
2639 putData);
2640 if (!SQL_SUCCEEDED(rc)) {
! 2641 LOG("SQLExecute: SQLPutData failed for SQL_C_WCHAR DAE streaming");
! 2642 return rc;
2643 }
2644 } else if (matchedInfo->paramCType == SQL_C_CHAR) {
2645 // Encode the string using the specified encoding
2646 std::string encodedStr;Lines 2732-2747 2732 SQLHANDLE hStmt = statementHandle->get();
2733
2734 // Configure forward-only / read-only cursor (matches slow path semantics).
2735 if (SQLSetStmtAttr_ptr) {
! 2736 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CURSOR_TYPE,
! 2737 (SQLPOINTER)SQL_CURSOR_FORWARD_ONLY, 0);
2738 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CONCURRENCY,
2739 (SQLPOINTER)SQL_CONCUR_READ_ONLY, 0);
! 2740 }
! 2741
! 2742 // The encoding-settings dict has the form {"encoding": str, "ctype": int}.
! 2743 // Note: the Python layer's SQL_C_CHAR constant is numerically -8, the same
2744 // as ODBC's SQL_C_WCHAR. As a result, the only path that genuinely uses
2745 // byte-level character encoding is when the user explicitly opts in via
2746 // setencoding(..., ctype=mssql_python.SQL_CHAR) (which sends ctype=1, the
2747 // real ODBC SQL_CHAR). We default to utf-8 and only honor the dict'sLines 2751-2767 2751 if (encoding_settings.contains("ctype") && encoding_settings.contains("encoding")) {
2752 int ctype = encoding_settings["ctype"].cast<int>();
2753 if (ctype == SQL_C_CHAR /* real ODBC value: 1 */) {
2754 charEncoding = encoding_settings["encoding"].cast<std::string>();
! 2755 }
! 2756 }
! 2757
! 2758 // The cursor.py caller always passes a fresh `list(actual_params)` so this
! 2759 // function is free to mutate slots in place. Even so, every site below uses
! 2760 // PyList_SetItem (which decrefs the old slot before stealing the new ref),
! 2761 // so the function is safe regardless of who owns the list.
! 2762
! 2763 // Run DetectParamTypes BEFORE SQLPrepare so that type-detection errors
2764 // (unsupported type, NaN Decimal, precision overflow) don't leave the
2765 // cursor in a half-prepared state.
2766 std::vector<ParamInfo> paramInfos = DetectParamTypes(params.ptr());Lines 2782-2791 2782 if (!SQL_SUCCEEDED(rc)) return rc;
2783 statementHandle->clearDescribeCache();
2784 is_stmt_prepared[0] = py::bool_(true);
2785 } else {
! 2786 ThrowStdException("Cannot execute unprepared statement");
! 2787 }
2788 }
2789
2790 std::vector<std::shared_ptr<void>> paramBuffers;
2791 rc = BindParameters(*statementHandle, hStmt, params, paramInfos, paramBuffers, charEncoding);Lines 2813-2823 2813 }
2814 if (rc != SQL_NEED_DATA) break;
2815
2816 const ParamInfo* matchedInfo = nullptr;
! 2817 for (auto& info : paramInfos) {
! 2818 if (reinterpret_cast<SQLPOINTER>(const_cast<ParamInfo*>(&info)) == paramToken) {
! 2819 matchedInfo = &info;
2820 break;
2821 }
2822 }
2823 if (!matchedInfo) {Lines 2825-2841 2825 }
2826 PyObject* pyObj = matchedInfo->dataPtr;
2827 if (!pyObj || pyObj == Py_None) {
2828 py::gil_scoped_release release;
! 2829 SQLPutData_ptr(hStmt, nullptr, 0);
! 2830 continue;
! 2831 }
! 2832
! 2833 if (PyUnicode_Check(pyObj)) {
2834 if (matchedInfo->paramCType == SQL_C_WCHAR) {
2835 std::u16string u16 =
2836 py::reinterpret_borrow<py::str>(py::handle(pyObj)).cast<std::u16string>();
! 2837 rc = stream_dae_chunks(
2838 reinterpretU16stringAsSqlWChar(u16),
2839 u16.size() * sizeof(SQLWCHAR),
2840 putData);
2841 if (!SQL_SUCCEEDED(rc)) return rc;Lines 2839-2847 2839 u16.size() * sizeof(SQLWCHAR),
2840 putData);
2841 if (!SQL_SUCCEEDED(rc)) return rc;
2842 } else if (matchedInfo->paramCType == SQL_C_CHAR) {
! 2843 std::string encodedStr;
2844 py::object encoded = py::reinterpret_borrow<py::object>(py::handle(pyObj))
2845 .attr("encode")(charEncoding, "strict");
2846 encodedStr = encoded.cast<std::string>();
2847 rc = stream_dae_chunks(encodedStr.data(), encodedStr.size(), putData);Lines 2859-2867 2859 if (PyBytes_Check(pyObj)) {
2860 bytesStorage = py::reinterpret_borrow<py::bytes>(py::handle(pyObj));
2861 dataPtr = bytesStorage.data();
2862 totalBytes = bytesStorage.size();
! 2863 } else {
2864 // bytearray is mutable — copy to stable buffer before streaming
2865 bytesStorage.assign(PyByteArray_AS_STRING(pyObj),
2866 static_cast<size_t>(PyByteArray_GET_SIZE(pyObj)));
2867 dataPtr = bytesStorage.data();Lines 2875-2886 2875 }
2876 }
2877 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;
2878 }
! 2879
! 2880 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;
! 2881
! 2882 // Unbind parameter buffers before they go out of scope.
2883 // Not called on error paths — diagnostics must remain readable.
2884 SQLRETURN exec_rc = rc;
2885 SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS);
2886 return exec_rc;Lines 3424-3432 3424
3425 // Get cached UUID class from module-level helper
3426 // This avoids static object destruction issues during
3427 // Python finalization
! 3428 py::object uuid_class = PythonObjectCache::get_uuid_class_obj();
3429 // Get cached UUID class
3430
3431 for (size_t i = 0; i < paramSetSize; ++i) {
3432 const py::handle& element = columnValues[i];Lines 5137-5145 5137 int totalMinutes = dtoValue.timezone_hour * 60 + dtoValue.timezone_minute;
5138 py::object datetime_module = py::module_::import("datetime");
5139 py::object tzinfo = datetime_module.attr("timezone")(
5140 datetime_module.attr("timedelta")(py::arg("minutes") = totalMinutes));
! 5141 py::object py_dt = PythonObjectCache::get_datetime_class_obj()(
5142 dtoValue.year, dtoValue.month, dtoValue.day, dtoValue.hour,
5143 dtoValue.minute, dtoValue.second,
5144 dtoValue.fraction / 1000, // ns → µs
5145 tzinfo);mssql_python/pybind/py_ref.hppLines 20-27 20 // Wrap a new reference (already +1 from the API that returned it).
21 inline PyPtr adopt(PyObject* p) noexcept { return PyPtr{p}; }
22
23 // Extend a borrowed reference's lifetime past its borrower.
! 24 inline PyPtr incref_borrow(PyObject* p) noexcept { Py_XINCREF(p); return PyPtr{p}; }
25
26 } // namespace pyref📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.py_ref.hpp: 66.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.connection.py: 83.6%
mssql_python.logging.py: 85.5%🔗 Quick Links
|
- Comment out use_prepare parameter name (C4100: unreferenced parameter) - Remove unused catch variable name (C4101: unreferenced local variable)
Add explicit null pointer and zero-length guards before memcpy in build_numeric_data to satisfy DevSkim code scanning rule DS121708.
…or 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.
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.
Resolve conflicts in ddbc_bindings.cpp from main's GH-610 work: - Keep both build_numeric_data (this PR) and ResolveNullParamType (main) - Adopt main's BindParameters/BindParameterArray signatures that take SqlHandle& handle; update the SQLExecuteFast_wrap call site to pass *statementHandle so the fast path uses the per-handle NULL describe cache - Migrate SQLExecuteFast_wrap from std::wstring + WStringToSQLWCHAR to std::u16string + reinterpretU16stringAsSqlWChar (main's uniform 16-bit query/param representation), dropping the platform #ifdef in both the prepare path and the DAE wide-char put-data loop Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 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>
…ny-perf-detect-types
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ecuteLegacy 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>
bewithgaurav
changed the title
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>
…ility pybind11's type_caster needs copy semantics for std::vector<ParamInfo>& in the legacy path. Provide a copy ctor that Py_XINCREFs dataPtr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
- 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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR shifts the primary cursor.execute() pipeline from Python into native C++ by adding a raw-CPython DetectParamTypes fast path and routing calls to a single DDBCSQLExecute FFI entrypoint when setinputsizes isn’t active, targeting large parameter-count performance regressions (GH-500).
Changes:
- Added a native
DetectParamTypes → BindParameters → SQLExecutepipeline (DDBCSQLExecute) to avoid per-parameter Python/pybind11 overhead. - Preserved a legacy path (
DDBCSQLExecuteLegacy) forsetinputsizesusers and updated Python routing accordingly. - Added parity and regression tests covering fast/slow path equivalence, subclass handling, DAE streaming, and refcount safety.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/test_023_fast_path_parity.py | Adds fast-vs-legacy parity tests and regression coverage for the new native execute pipeline. |
| tests/test_010_pybind_functions.py | Updates exposed-function expectations to include DDBCSQLExecuteLegacy. |
| mssql_python/pybind/ddbc_bindings.cpp | Implements native type detection, new execute entrypoints, and various binding/DAE handling updates. |
| mssql_python/cursor.py | Routes execute() to DDBCSQLExecute for the primary path and to DDBCSQLExecuteLegacy when setinputsizes overrides are present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2772
to
+2775
| } else { | ||
| dataPtr = PyByteArray_AS_STRING(pyObj); | ||
| totalBytes = static_cast<size_t>(PyByteArray_GET_SIZE(pyObj)); | ||
| } |
…E safety - 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>
- 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>
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>
…_class, import_attr - 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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
Introduce py_ref.hpp: PyPtr = std::unique_ptr<PyObject, PyDecRefDeleter> 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
Moves parameter type detection and binding from Python into a native C++ pipeline using raw CPython API calls. The new
DDBCSQLExecutehandles type detection → parameter binding → SQLExecute in a single FFI crossing, eliminating per-parameter Python overhead entirely.What changed:
DetectParamTypes— C++ type detection using raw CPython API (PyLong_Check,PyDateTime_Check,PyObject_RichCompareBool, etc.) replacing the Python-side_create_parameter_types_listloop for the primary execute path.DDBCSQLExecute(formerlyDDBCSQLExecuteFast) — single C++ pipeline: detect → bind → execute. ParamInfo never crosses the pybind11 boundary.DDBCSQLExecuteLegacy(formerlyDDBCSQLExecute) — retained forsetinputsizesusers only.PythonObjectCachestores all type objects as rawPyObject*(notpy::object), eliminating pybind11 wrapper overhead on every cache hit.Py_DECREFcleanup andPyObject_IsInstanceerror handling on all paths.Routing (cursor.py):
Performance Results 🚀
The Python-side type detection cost was ~2.0–2.3µs per parameter — an
isinstancecheck,ParamInfoobject construction, and a pybind11 FFI boundary crossing per parameter, per execute call. The C++ path replaces this with ~35ns/param (rawPyLong_Check+ struct field write) — a ~60x faster per-parameter detection.macOS arm64 (Apple Silicon M-series), Python 3.13
Linux aarch64 (Docker container), Python 3.13
vs pyodbc (post-PR, macOS)
Bottom line
Checklist