PERF: Native C++ parameter detection and execute pipeline - #549
Open
bewithgaurav wants to merge 33 commits into
Open
PERF: Native C++ parameter detection and execute pipeline#549bewithgaurav wants to merge 33 commits into
bewithgaurav wants to merge 33 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 149-195 149 Py_XINCREF(dataPtr);
150 }
151 // Copy/move assignment and move constructor below are required for
152 // std::vector<ParamInfo> resize/reallocation and pybind11's type_caster.
! 153 // They manually manage dataPtr refcounts since ParamInfo owns a strong ref.
! 154 ParamInfo& operator=(const ParamInfo& other) {
! 155 if (this != &other) {
! 156 Py_XDECREF(dataPtr);
! 157 inputOutputType = other.inputOutputType;
! 158 paramCType = other.paramCType;
! 159 paramSQLType = other.paramSQLType;
! 160 columnSize = other.columnSize;
! 161 decimalDigits = other.decimalDigits;
! 162 strLenOrInd = other.strLenOrInd;
! 163 isDAE = other.isDAE;
! 164 dataPtr = other.dataPtr;
! 165 utf16Len = other.utf16Len;
! 166 Py_XINCREF(dataPtr);
! 167 }
! 168 return *this;
169 }
! 170 ParamInfo(ParamInfo&& other) noexcept
! 171 : inputOutputType(other.inputOutputType), paramCType(other.paramCType),
! 172 paramSQLType(other.paramSQLType), columnSize(other.columnSize),
! 173 decimalDigits(other.decimalDigits), strLenOrInd(other.strLenOrInd),
! 174 isDAE(other.isDAE), dataPtr(other.dataPtr), utf16Len(other.utf16Len) {
! 175 other.dataPtr = nullptr;
! 176 }
! 177 ParamInfo& operator=(ParamInfo&& other) noexcept {
! 178 if (this != &other) {
! 179 Py_XDECREF(dataPtr);
! 180 inputOutputType = other.inputOutputType;
! 181 paramCType = other.paramCType;
! 182 paramSQLType = other.paramSQLType;
! 183 columnSize = other.columnSize;
! 184 decimalDigits = other.decimalDigits;
! 185 strLenOrInd = other.strLenOrInd;
! 186 isDAE = other.isDAE;
! 187 dataPtr = other.dataPtr;
! 188 utf16Len = other.utf16Len;
! 189 other.dataPtr = nullptr;
! 190 }
! 191 return *this;
192 }
193 };
194 #ifdef __GNUC__
195 #pragma GCC diagnostic popLines 396-404 396 SQLDescribeParamFunc SQLDescribeParam_ptr = nullptr;
397
398 namespace {
399
! 400
401 const char* GetSqlCTypeAsString(const SQLSMALLINT cType) {
402 switch (cType) {
403 STRINGIFY_FOR_CASE(SQL_C_CHAR);
404 STRINGIFY_FOR_CASE(SQL_C_WCHAR);Lines 721-730 721 Py_ssize_t time_len = PyUnicode_GET_LENGTH(time_str);
722 info.columnSize = std::max<SQLULEN>(info.columnSize, time_len);
723 // PyList_SetItem (lowercase) decrefs the old slot before stealing the new
724 // reference; safe here because cursor.py already passed a fresh list copy.
! 725 if (PyList_SetItem(params, i, time_str) != 0) {
! 726 throw py::error_already_set();
727 }
728 continue;
729 }Lines 746-755 746
747 py::object digits_obj = steal(PyObject_GetAttrString(as_tuple_ptr.ptr(), "digits"));
748 if (!digits_obj) throw py::error_already_set();
749
! 750 if (!PyTuple_Check(digits_obj.ptr())) {
! 751 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
752 }
753
754 Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj.ptr());
755 int exponent = static_cast<int>(PyLong_AsLong(exponent_obj.ptr()));Lines 804-813 804 info.decimalDigits = 0;
805 PyObject* raw = formatted.release().ptr();
806 if (PyList_SetItem(params, i, raw) != 0) {
807 // PyList_SetItem steals (decrefs) the item even on failure,
! 808 // so raw is already freed — do NOT Py_DECREF here.
! 809 throw py::error_already_set();
810 }
811 continue;
812 }Lines 821-830 821 // Store NumericData as a Python object in the param list for the binder.
822 py::object numeric_obj = py::cast(nd);
823 PyObject* raw = numeric_obj.release().ptr();
824 if (PyList_SetItem(params, i, raw) != 0) {
! 825 // PyList_SetItem steals (decrefs) the item even on failure.
! 826 throw py::error_already_set();
827 }
828 continue;
829 }Lines 838-847 838 info.paramCType = SQL_C_GUID;
839 info.columnSize = 16;
840 info.decimalDigits = 0;
841 if (PyList_SetItem(params, i, bytes_le) != 0) {
! 842 // PyList_SetItem steals (decrefs) the item even on failure.
! 843 throw py::error_already_set();
844 }
845 continue;
846 }Lines 870-879 870 if (!sign_obj) throw py::error_already_set();
871 int sign_val = static_cast<int>(PyLong_AsLong(sign_obj.ptr()));
872 if (sign_val == -1 && PyErr_Occurred()) throw py::error_already_set();
873
! 874 if (!PyTuple_Check(digits)) {
! 875 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
876 }
877
878 // SQL Server precision counts all stored decimal digits, while scale is just the
879 // fractional digits. A positive exponent moves trailing zeros into the integer part;Lines 916-925 916 // must become mantissa 12300 before packing.
917 for (int j = 0; j < exponent; ++j) {
918 overflow |= mul10_add(0);
919 }
! 920 if (overflow != 0) {
! 921 throw py::value_error("Decimal magnitude exceeds the 16-byte SQL NUMERIC capacity");
922 }
923
924 NumericData nd;
925 nd.precision = static_cast<SQLCHAR>(precision);Lines 1192-1201 1192 dataPtr = sqlwcharBuffer->data();
1193 bufferLength = sqlwcharBuffer->size() * sizeof(SQLWCHAR);
1194 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
1195 // Use explicit byte length instead of SQL_NTS so embedded NUL chars
! 1196 // aren't treated as string terminators.
! 1197 *strLenOrIndPtr = static_cast<SQLLEN>(sqlwcharBuffer->size() * sizeof(SQLWCHAR));
1198 }
1199 break;
1200 }
1201 case SQL_C_BIT: {Lines 1334-1342 1334 dataPtr = static_cast<void*>(sqlTimePtr);
1335 break;
1336 }
1337 case SQL_C_SS_TIMESTAMPOFFSET: {
! 1338 py::object datetimeType = PyTypeCache::get_datetime_class_obj();
1339 if (!py::isinstance(param, datetimeType)) {
1340 ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
1341 }
1342 // Checking if the object has a timezoneLines 2527-2536 2527 continue;
2528 }
2529 if (PyUnicode_Check(pyObj)) {
2530 if (matchedInfo->paramCType == SQL_C_WCHAR) {
! 2531 std::u16string utf16 =
! 2532 py::reinterpret_borrow<py::str>(py::handle(pyObj)).cast<std::u16string>();
2533 rc = stream_dae_chunks(
2534 reinterpretU16stringAsSqlWChar(utf16),
2535 utf16.size() * sizeof(SQLWCHAR),
2536 putData);Lines 2622-2637 2622 py::list is_stmt_prepared,
2623 bool use_prepare,
2624 const py::dict& encoding_settings) {
2625 if (!statementHandle || !statementHandle->get()) {
! 2626 return SQL_INVALID_HANDLE;
! 2627 }
2628
2629 SQLHANDLE hStmt = statementHandle->get();
! 2630
! 2631 // Configure forward-only / read-only cursor (matches slow path semantics).
! 2632 if (SQLSetStmtAttr_ptr) {
! 2633 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CURSOR_TYPE,
2634 (SQLPOINTER)SQL_CURSOR_FORWARD_ONLY, 0);
2635 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CONCURRENCY,
2636 (SQLPOINTER)SQL_CONCUR_READ_ONLY, 0);
2637 }Lines 2641-2657 2641 // as ODBC's SQL_C_WCHAR. As a result, the only path that genuinely uses
2642 // byte-level character encoding is when the user explicitly opts in via
2643 // setencoding(..., ctype=mssql_python.SQL_CHAR) (which sends ctype=1, the
2644 // real ODBC SQL_CHAR). We default to utf-8 and only honor the dict's
! 2645 // encoding when ctype == 1 (real ODBC SQL_CHAR). Otherwise the user's
! 2646 // "encoding" value is meant for the wide-char path and we leave it alone.
! 2647 std::string charEncoding = "utf-8";
! 2648 if (encoding_settings.contains("ctype") && encoding_settings.contains("encoding")) {
! 2649 int ctype = encoding_settings["ctype"].cast<int>();
! 2650 if (ctype == SQL_C_CHAR /* real ODBC value: 1 */) {
! 2651 charEncoding = encoding_settings["encoding"].cast<std::string>();
! 2652 }
! 2653 }
2654
2655 // The cursor.py caller always passes a fresh `list(actual_params)` so this
2656 // function is free to mutate slots in place. Even so, every site below uses
2657 // PyList_SetItem (which decrefs the old slot before stealing the new ref),Lines 2672-2681 2672 if (!already_prepared) {
2673 if (use_prepare) {
2674 SQLWCHAR* queryPtr = reinterpretU16stringAsSqlWChar(query);
2675 {
! 2676 py::gil_scoped_release release;
! 2677 rc = SQLPrepare_ptr(hStmt, queryPtr, SQL_NTS);
2678 }
2679 if (!SQL_SUCCEEDED(rc)) return rc;
2680 statementHandle->clearDescribeCache();
2681 is_stmt_prepared[0] = py::bool_(true);Lines 2703-2713 2703 py::gil_scoped_release release;
2704 return SQLPutData_ptr(hStmt, data, len);
2705 };
2706 while (true) {
! 2707 {
! 2708 py::gil_scoped_release release;
! 2709 rc = SQLParamData_ptr(hStmt, ¶mToken);
2710 }
2711 if (rc != SQL_NEED_DATA) break;
2712
2713 const ParamInfo* matchedInfo = nullptr;Lines 2715-2731 2715 if (reinterpret_cast<SQLPOINTER>(const_cast<ParamInfo*>(&info)) == paramToken) {
2716 matchedInfo = &info;
2717 break;
2718 }
! 2719 }
! 2720 if (!matchedInfo) {
! 2721 ThrowStdException("SQLExecuteFast: unrecognized paramToken from SQLParamData");
! 2722 }
! 2723 PyObject* pyObj = matchedInfo->dataPtr;
2724 if (!pyObj || pyObj == Py_None) {
2725 py::gil_scoped_release release;
2726 SQLPutData_ptr(hStmt, nullptr, 0);
! 2727 continue;
2728 }
2729
2730 if (PyUnicode_Check(pyObj)) {
2731 if (matchedInfo->paramCType == SQL_C_WCHAR) {Lines 2729-2737 2729
2730 if (PyUnicode_Check(pyObj)) {
2731 if (matchedInfo->paramCType == SQL_C_WCHAR) {
2732 std::u16string u16 =
! 2733 py::reinterpret_borrow<py::str>(py::handle(pyObj)).cast<std::u16string>();
2734 rc = stream_dae_chunks(
2735 reinterpretU16stringAsSqlWChar(u16),
2736 u16.size() * sizeof(SQLWCHAR),
2737 putData);Lines 2749-2757 2749 } else if (PyBytes_Check(pyObj) || PyByteArray_Check(pyObj)) {
2750 // Handle bytes and bytearray separately — pybind11's bytes
2751 // caster does not safely convert bytearray.
2752 const char* dataPtr = nullptr;
! 2753 size_t totalBytes = 0;
2754 std::string bytesStorage; // lifetime must span the loop
2755
2756 if (PyBytes_Check(pyObj)) {
2757 bytesStorage = py::reinterpret_borrow<py::bytes>(py::handle(pyObj));Lines 2765-2776 2765 totalBytes = bytesStorage.size();
2766 }
2767
2768 rc = stream_dae_chunks(dataPtr, totalBytes, putData);
! 2769 if (!SQL_SUCCEEDED(rc)) return rc;
! 2770 } else {
! 2771 ThrowStdException("SQLExecuteFast: DAE only supported for str or bytes");
! 2772 }
2773 }
2774 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;
2775 }Lines 2778-2786 2778
2779 // Unbind parameter buffers before they go out of scope.
2780 // Not called on error paths — diagnostics must remain readable.
2781 SQLRETURN exec_rc = rc;
! 2782 SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS);
2783 return exec_rc;
2784 }
2785
2786 SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list& columnwise_params,Lines 3321-3329 3321
3322 // Get cached UUID class from module-level helper
3323 // This avoids static object destruction issues during
3324 // Python finalization
! 3325 py::object uuid_class = PyTypeCache::get_uuid_class_obj();
3326 // Get cached UUID class
3327
3328 for (size_t i = 0; i < paramSetSize; ++i) {
3329 const py::handle& element = columnValues[i];Lines 4410-4418 4410 int microseconds = dtoValue.fraction / 1000;
4411 py::object datetime_module = py::module_::import("datetime");
4412 py::object tzinfo = datetime_module.attr("timezone")(
4413 datetime_module.attr("timedelta")(py::arg("minutes") = totalMinutes));
! 4414 py::object py_dt = PyTypeCache::get_datetime_class_obj()(
4415 dtoValue.year, dtoValue.month, dtoValue.day, dtoValue.hour, dtoValue.minute,
4416 dtoValue.second, microseconds, tzinfo);
4417 row.append(py_dt);
4418 } else {mssql_python/pybind/py_type_cache.hppLines 43-54 43 // Return cached type, falling back to import for legacy paths that call
44 // before initialize() has run.
45 inline PyObject* get_cached_class(PyObject* cached, const char* module_name, const char* attr_name) {
46 if (cache_initialized && cached) return cached;
! 47 py::object mod = steal(PyImport_ImportModule(module_name));
! 48 if (!mod) return nullptr;
! 49 return PyObject_GetAttrString(mod.ptr(), attr_name);
! 50 }
51
52 // One-time init. Uses local py::objects so exception cleanup is automatic;
53 // only .release() into globals after ALL acquisitions succeed.
54 inline void initialize() {📋 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.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.1%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 83.7%
mssql_python.connection.py: 84.7%🔗 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.
…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>
… bewithgaurav/insertmany-perf-detect-types
…le-free bugs - Extract PythonObjectCache namespace to python_object_cache.hpp - Extend PyPtr usage to Groups A/B/D/E (22→7 manual decrefs) - Fix 3 double-free bugs in PyList_SetItem error paths - Document ParamInfo.dataPtr manual refcounting rationale Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename python_object_cache.hpp → py_type_cache.hpp - Rename namespace PythonObjectCache → PyTypeCache - Remove unused: incref_borrow using, <cctype>, <iomanip> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…om PyPtr drops py_ref.hpp (PyPtr = unique_ptr<PyObject, PyDecRefDeleter>) and uses pybind11's own RAII handle via a steal() shorthand. same ownership semantics, no behavior change: in release builds (-O3 -DNDEBUG, which is what we ship) py::object::dec_ref compiles to a bare Py_XDECREF, identical to the PyPtr deleter. the raw CPython calls in the hot detection path are untouched, only the refcount wrapper changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… bigint build_numeric_data walked every Decimal digit through PyNumber_Multiply/PyNumber_Add, allocating three PyLongs per digit, then called PyNumber_Absolute and to_bytes(16) to serialise. it also re-entered Python for as_tuple() and the digits/exponent attributes that DetectParamTypes had already fetched. SQL Server caps NUMERIC at 38 digits and callers reject anything larger, so the mantissa always fits 128 bits. accumulate it in four uint32 limbs and write the 16 little-endian bytes directly. limbs rather than __int128 because MSVC has no __int128, and the bytes are written explicitly so host endianness does not matter. as_tuple/digits/exponent are now passed in from the caller. numeric decimal path measured 2.3x-2.9x faster (19-digit 1643 -> 711 ns/param, 38-digit 2401 -> 824). money path and all non-decimal types unchanged. 1922 tests pass, and decimal round-trip verified across sign, zero, money bounds, positive exponents, 38-digit magnitudes and scale-38 values. 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