PERF: Native C++ parameter detection and execute pipeline#549
Open
bewithgaurav wants to merge 31 commits into
Open
PERF: Native C++ parameter detection and execute pipeline#549bewithgaurav wants to merge 31 commits into
bewithgaurav wants to merge 31 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 154-200 154 Py_XINCREF(dataPtr);
155 }
156 // Copy/move assignment and move constructor below are required for
157 // std::vector<ParamInfo> resize/reallocation and pybind11's type_caster.
! 158 // They manually manage dataPtr refcounts since ParamInfo owns a strong ref.
! 159 ParamInfo& operator=(const ParamInfo& other) {
! 160 if (this != &other) {
! 161 Py_XDECREF(dataPtr);
! 162 inputOutputType = other.inputOutputType;
! 163 paramCType = other.paramCType;
! 164 paramSQLType = other.paramSQLType;
! 165 columnSize = other.columnSize;
! 166 decimalDigits = other.decimalDigits;
! 167 strLenOrInd = other.strLenOrInd;
! 168 isDAE = other.isDAE;
! 169 dataPtr = other.dataPtr;
! 170 utf16Len = other.utf16Len;
! 171 Py_XINCREF(dataPtr);
! 172 }
! 173 return *this;
174 }
! 175 ParamInfo(ParamInfo&& other) noexcept
! 176 : inputOutputType(other.inputOutputType), paramCType(other.paramCType),
! 177 paramSQLType(other.paramSQLType), columnSize(other.columnSize),
! 178 decimalDigits(other.decimalDigits), strLenOrInd(other.strLenOrInd),
! 179 isDAE(other.isDAE), dataPtr(other.dataPtr), utf16Len(other.utf16Len) {
! 180 other.dataPtr = nullptr;
! 181 }
! 182 ParamInfo& operator=(ParamInfo&& other) noexcept {
! 183 if (this != &other) {
! 184 Py_XDECREF(dataPtr);
! 185 inputOutputType = other.inputOutputType;
! 186 paramCType = other.paramCType;
! 187 paramSQLType = other.paramSQLType;
! 188 columnSize = other.columnSize;
! 189 decimalDigits = other.decimalDigits;
! 190 strLenOrInd = other.strLenOrInd;
! 191 isDAE = other.isDAE;
! 192 dataPtr = other.dataPtr;
! 193 utf16Len = other.utf16Len;
! 194 other.dataPtr = nullptr;
! 195 }
! 196 return *this;
197 }
198 };
199 #ifdef __GNUC__
200 #pragma GCC diagnostic popLines 401-409 401 SQLDescribeParamFunc SQLDescribeParam_ptr = nullptr;
402
403 namespace {
404
! 405
406 const char* GetSqlCTypeAsString(const SQLSMALLINT cType) {
407 switch (cType) {
408 STRINGIFY_FOR_CASE(SQL_C_CHAR);
409 STRINGIFY_FOR_CASE(SQL_C_WCHAR);Lines 726-735 726 Py_ssize_t time_len = PyUnicode_GET_LENGTH(time_str);
727 info.columnSize = std::max<SQLULEN>(info.columnSize, time_len);
728 // PyList_SetItem (lowercase) decrefs the old slot before stealing the new
729 // reference; safe here because cursor.py already passed a fresh list copy.
! 730 if (PyList_SetItem(params, i, time_str) != 0) {
! 731 throw py::error_already_set();
732 }
733 continue;
734 }Lines 751-760 751
752 PyPtr digits_obj = adopt(PyObject_GetAttrString(as_tuple_ptr.get(), "digits"));
753 if (!digits_obj) throw py::error_already_set();
754
! 755 if (!PyTuple_Check(digits_obj.get())) {
! 756 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
757 }
758
759 Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj.get());
760 int exponent = static_cast<int>(PyLong_AsLong(exponent_obj.get()));Lines 809-818 809 info.decimalDigits = 0;
810 PyObject* raw = formatted.release();
811 if (PyList_SetItem(params, i, raw) != 0) {
812 // PyList_SetItem steals (decrefs) the item even on failure,
! 813 // so raw is already freed — do NOT Py_DECREF here.
! 814 throw py::error_already_set();
815 }
816 continue;
817 }Lines 826-835 826 // Store NumericData as a Python object in the param list for the binder.
827 py::object numeric_obj = py::cast(nd);
828 PyObject* raw = numeric_obj.release().ptr();
829 if (PyList_SetItem(params, i, raw) != 0) {
! 830 // PyList_SetItem steals (decrefs) the item even on failure.
! 831 throw py::error_already_set();
832 }
833 continue;
834 }Lines 843-852 843 info.paramCType = SQL_C_GUID;
844 info.columnSize = 16;
845 info.decimalDigits = 0;
846 if (PyList_SetItem(params, i, bytes_le) != 0) {
! 847 // PyList_SetItem steals (decrefs) the item even on failure.
! 848 throw py::error_already_set();
849 }
850 continue;
851 }Lines 886-895 886 if (exponent == -1 && PyErr_Occurred()) throw py::error_already_set();
887 }
888 exponent_obj.reset();
889
! 890 if (!PyTuple_Check(digits.get())) {
! 891 throw py::type_error("Decimal.as_tuple().digits must be a tuple");
892 }
893
894 // Step 5: SQL Server precision counts all stored decimal digits, while scale is just the fractional
895 // digits. A positive exponent means trailing zeros move into the integer part; a negative exponentLines 909-918 909 // Step 6: build the mantissa with Python bigint math so we preserve every Decimal digit; a fixed
910 // C++ integer would overflow long before SQL Server's 38-digit NUMERIC limit.
911 PyPtr py_ten = adopt(PyLong_FromLong(10));
912 PyPtr int_val = adopt(PyLong_FromLong(0));
! 913 if (!py_ten || !int_val) {
! 914 throw py::error_already_set();
915 }
916
917 const Py_ssize_t digit_count = PyTuple_GET_SIZE(digits.get());
918 for (Py_ssize_t i = 0; i < digit_count; ++i) {Lines 958-967 958 if (!val_bytes) throw py::error_already_set();
959
960 char* val_buf = nullptr;
961 Py_ssize_t val_size = 0;
! 962 if (PyBytes_AsStringAndSize(val_bytes.get(), &val_buf, &val_size) == -1) {
! 963 throw py::error_already_set();
964 }
965
966 // Step 10: pack precision/scale plus the 16-byte little-endian magnitude. SQL uses sign=1 for
967 // positive and sign=0 for negative, which is the inverse of Decimal.as_tuple().sign.Lines 1237-1246 1237 dataPtr = sqlwcharBuffer->data();
1238 bufferLength = sqlwcharBuffer->size() * sizeof(SQLWCHAR);
1239 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
1240 // Use explicit byte length instead of SQL_NTS so embedded NUL chars
! 1241 // aren't treated as string terminators.
! 1242 *strLenOrIndPtr = static_cast<SQLLEN>(sqlwcharBuffer->size() * sizeof(SQLWCHAR));
1243 }
1244 break;
1245 }
1246 case SQL_C_BIT: {Lines 1379-1387 1379 dataPtr = static_cast<void*>(sqlTimePtr);
1380 break;
1381 }
1382 case SQL_C_SS_TIMESTAMPOFFSET: {
! 1383 py::object datetimeType = PyTypeCache::get_datetime_class_obj();
1384 if (!py::isinstance(param, datetimeType)) {
1385 ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
1386 }
1387 // Checking if the object has a timezoneLines 2475-2484 2475 reinterpretU16stringAsSqlWChar(utf16),
2476 utf16.size() * sizeof(SQLWCHAR),
2477 putData);
2478 if (!SQL_SUCCEEDED(rc)) {
! 2479 LOG("SQLExecute: SQLPutData failed for SQL_C_WCHAR DAE streaming");
! 2480 return rc;
2481 }
2482 } else if (matchedInfo->paramCType == SQL_C_CHAR) {
2483 // Encode the string using the specified encoding
2484 std::string encodedStr;Lines 2570-2585 2570 SQLHANDLE hStmt = statementHandle->get();
2571
2572 // Configure forward-only / read-only cursor (matches slow path semantics).
2573 if (SQLSetStmtAttr_ptr) {
! 2574 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CURSOR_TYPE,
! 2575 (SQLPOINTER)SQL_CURSOR_FORWARD_ONLY, 0);
2576 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CONCURRENCY,
2577 (SQLPOINTER)SQL_CONCUR_READ_ONLY, 0);
! 2578 }
! 2579
! 2580 // The encoding-settings dict has the form {"encoding": str, "ctype": int}.
! 2581 // Note: the Python layer's SQL_C_CHAR constant is numerically -8, the same
2582 // as ODBC's SQL_C_WCHAR. As a result, the only path that genuinely uses
2583 // byte-level character encoding is when the user explicitly opts in via
2584 // setencoding(..., ctype=mssql_python.SQL_CHAR) (which sends ctype=1, the
2585 // real ODBC SQL_CHAR). We default to utf-8 and only honor the dict'sLines 2589-2605 2589 if (encoding_settings.contains("ctype") && encoding_settings.contains("encoding")) {
2590 int ctype = encoding_settings["ctype"].cast<int>();
2591 if (ctype == SQL_C_CHAR /* real ODBC value: 1 */) {
2592 charEncoding = encoding_settings["encoding"].cast<std::string>();
! 2593 }
! 2594 }
! 2595
! 2596 // The cursor.py caller always passes a fresh `list(actual_params)` so this
! 2597 // function is free to mutate slots in place. Even so, every site below uses
! 2598 // PyList_SetItem (which decrefs the old slot before stealing the new ref),
! 2599 // so the function is safe regardless of who owns the list.
! 2600
! 2601 // Run DetectParamTypes BEFORE SQLPrepare so that type-detection errors
2602 // (unsupported type, NaN Decimal, precision overflow) don't leave the
2603 // cursor in a half-prepared state.
2604 std::vector<ParamInfo> paramInfos = DetectParamTypes(params.ptr());Lines 2620-2629 2620 if (!SQL_SUCCEEDED(rc)) return rc;
2621 statementHandle->clearDescribeCache();
2622 is_stmt_prepared[0] = py::bool_(true);
2623 } else {
! 2624 ThrowStdException("Cannot execute unprepared statement");
! 2625 }
2626 }
2627
2628 std::vector<std::shared_ptr<void>> paramBuffers;
2629 rc = BindParameters(*statementHandle, hStmt, params, paramInfos, paramBuffers, charEncoding);Lines 2651-2661 2651 }
2652 if (rc != SQL_NEED_DATA) break;
2653
2654 const ParamInfo* matchedInfo = nullptr;
! 2655 for (auto& info : paramInfos) {
! 2656 if (reinterpret_cast<SQLPOINTER>(const_cast<ParamInfo*>(&info)) == paramToken) {
! 2657 matchedInfo = &info;
2658 break;
2659 }
2660 }
2661 if (!matchedInfo) {Lines 2663-2679 2663 }
2664 PyObject* pyObj = matchedInfo->dataPtr;
2665 if (!pyObj || pyObj == Py_None) {
2666 py::gil_scoped_release release;
! 2667 SQLPutData_ptr(hStmt, nullptr, 0);
! 2668 continue;
! 2669 }
! 2670
! 2671 if (PyUnicode_Check(pyObj)) {
2672 if (matchedInfo->paramCType == SQL_C_WCHAR) {
2673 std::u16string u16 =
2674 py::reinterpret_borrow<py::str>(py::handle(pyObj)).cast<std::u16string>();
! 2675 rc = stream_dae_chunks(
2676 reinterpretU16stringAsSqlWChar(u16),
2677 u16.size() * sizeof(SQLWCHAR),
2678 putData);
2679 if (!SQL_SUCCEEDED(rc)) return rc;Lines 2677-2685 2677 u16.size() * sizeof(SQLWCHAR),
2678 putData);
2679 if (!SQL_SUCCEEDED(rc)) return rc;
2680 } else if (matchedInfo->paramCType == SQL_C_CHAR) {
! 2681 std::string encodedStr;
2682 py::object encoded = py::reinterpret_borrow<py::object>(py::handle(pyObj))
2683 .attr("encode")(charEncoding, "strict");
2684 encodedStr = encoded.cast<std::string>();
2685 rc = stream_dae_chunks(encodedStr.data(), encodedStr.size(), putData);Lines 2697-2705 2697 if (PyBytes_Check(pyObj)) {
2698 bytesStorage = py::reinterpret_borrow<py::bytes>(py::handle(pyObj));
2699 dataPtr = bytesStorage.data();
2700 totalBytes = bytesStorage.size();
! 2701 } else {
2702 // bytearray is mutable — copy to stable buffer before streaming
2703 bytesStorage.assign(PyByteArray_AS_STRING(pyObj),
2704 static_cast<size_t>(PyByteArray_GET_SIZE(pyObj)));
2705 dataPtr = bytesStorage.data();Lines 2713-2724 2713 }
2714 }
2715 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;
2716 }
! 2717
! 2718 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;
! 2719
! 2720 // Unbind parameter buffers before they go out of scope.
2721 // Not called on error paths — diagnostics must remain readable.
2722 SQLRETURN exec_rc = rc;
2723 SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS);
2724 return exec_rc;Lines 3262-3270 3262
3263 // Get cached UUID class from module-level helper
3264 // This avoids static object destruction issues during
3265 // Python finalization
! 3266 py::object uuid_class = PyTypeCache::get_uuid_class_obj();
3267 // Get cached UUID class
3268
3269 for (size_t i = 0; i < paramSetSize; ++i) {
3270 const py::handle& element = columnValues[i];Lines 4975-4983 4975 int totalMinutes = dtoValue.timezone_hour * 60 + dtoValue.timezone_minute;
4976 py::object datetime_module = py::module_::import("datetime");
4977 py::object tzinfo = datetime_module.attr("timezone")(
4978 datetime_module.attr("timedelta")(py::arg("minutes") = totalMinutes));
! 4979 py::object py_dt = PyTypeCache::get_datetime_class_obj()(
4980 dtoValue.year, dtoValue.month, dtoValue.day, dtoValue.hour,
4981 dtoValue.minute, dtoValue.second,
4982 dtoValue.fraction / 1000, // ns → µs
4983 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 pyrefmssql_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 PyPtr mod = adopt(PyImport_ImportModule(module_name));
! 48 if (!mod) return nullptr;
! 49 return PyObject_GetAttrString(mod.get(), attr_name);
! 50 }
51
52 // One-time init. Uses local PyPtrs 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.py_ref.hpp: 66.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.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%🔗 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>
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