PERF: Native C++ parameter detection and execute pipeline#549
Open
bewithgaurav wants to merge 22 commits into
Open
PERF: Native C++ parameter detection and execute pipeline#549bewithgaurav wants to merge 22 commits into
bewithgaurav wants to merge 22 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 136-145 136 // unnecessary ref-count traffic on every parameter.
137 void initialize() {
138 if (!cache_initialized) {
139 PyDateTime_IMPORT;
! 140 if (PyDateTimeAPI == nullptr) {
! 141 throw py::error_already_set();
142 }
143
144 // Cache datetime.datetime, datetime.date, datetime.time for isinstance
145 // checks in DetectParamTypes. Single import, then borrowed refs heldLines 149-157 149 datetime_class = PyObject_GetAttrString(datetime_module, "datetime");
150 date_class = PyObject_GetAttrString(datetime_module, "date");
151 time_class = PyObject_GetAttrString(datetime_module, "time");
152 Py_DECREF(datetime_module);
! 153 if (!datetime_class || !date_class || !time_class)
154 throw py::error_already_set();
155
156 PyObject* decimal_module = PyImport_ImportModule("decimal");
157 if (!decimal_module) throw py::error_already_set();Lines 174-191 174 smallmoney_max = PyObject_CallFunction(Decimal, "s", "214748.3647");
175 money_min = PyObject_CallFunction(Decimal, "s", "-922337203685477.5808");
176 money_max = PyObject_CallFunction(Decimal, "s", "922337203685477.5807");
177 if (!smallmoney_min || !smallmoney_max || !money_min || !money_max) {
! 178 // Partial creation — clean up whichever succeeded before throwing.
! 179 Py_XDECREF(smallmoney_min);
! 180 Py_XDECREF(smallmoney_max);
! 181 Py_XDECREF(money_min);
! 182 Py_XDECREF(money_max);
! 183 smallmoney_min = nullptr;
! 184 smallmoney_max = nullptr;
! 185 money_min = nullptr;
! 186 money_max = nullptr;
! 187 throw py::error_already_set();
188 }
189
190 cache_initialized = true;
191 }Lines 204-216 204 PyObject* get_datetime_class() {
205 if (cache_initialized && datetime_class) {
206 return datetime_class;
207 }
! 208 PyObject* mod = PyImport_ImportModule("datetime");
! 209 if (!mod) return nullptr;
! 210 PyObject* cls = PyObject_GetAttrString(mod, "datetime");
! 211 Py_DECREF(mod);
! 212 return cls; // caller must check for NULL
213 }
214
215 py::object get_datetime_class_obj() {
216 return wrap_cached_or_imported(get_datetime_class());Lines 221-233 221 PyObject* get_date_class() {
222 if (cache_initialized && date_class) {
223 return date_class;
224 }
! 225 PyObject* mod = PyImport_ImportModule("datetime");
! 226 if (!mod) return nullptr;
! 227 PyObject* cls = PyObject_GetAttrString(mod, "date");
! 228 Py_DECREF(mod);
! 229 return cls; // caller must check for NULL
230 }
231
232 py::object get_date_class_obj() {
233 return wrap_cached_or_imported(get_date_class());Lines 238-250 238 PyObject* get_time_class() {
239 if (cache_initialized && time_class) {
240 return time_class;
241 }
! 242 PyObject* mod = PyImport_ImportModule("datetime");
! 243 if (!mod) return nullptr;
! 244 PyObject* cls = PyObject_GetAttrString(mod, "time");
! 245 Py_DECREF(mod);
! 246 return cls; // caller must check for NULL
247 }
248
249 py::object get_time_class_obj() {
250 return wrap_cached_or_imported(get_time_class());Lines 255-267 255 PyObject* get_decimal_class() {
256 if (cache_initialized && decimal_class) {
257 return decimal_class;
258 }
! 259 PyObject* mod = PyImport_ImportModule("decimal");
! 260 if (!mod) return nullptr;
! 261 PyObject* cls = PyObject_GetAttrString(mod, "Decimal");
! 262 Py_DECREF(mod);
! 263 return cls; // caller must check for NULL
264 }
265
266 py::object get_decimal_class_obj() {
267 return wrap_cached_or_imported(get_decimal_class());Lines 272-284 272 PyObject* get_uuid_class() {
273 if (cache_initialized && uuid_class) {
274 return uuid_class;
275 }
! 276 PyObject* mod = PyImport_ImportModule("uuid");
! 277 if (!mod) return nullptr;
! 278 PyObject* cls = PyObject_GetAttrString(mod, "UUID");
! 279 Py_DECREF(mod);
! 280 return cls; // caller must check for NULL
281 }
282
283 py::object get_uuid_class_obj() {
284 return wrap_cached_or_imported(get_uuid_class());Lines 322-368 322 Py_XINCREF(dataPtr);
323 }
324 // Copy/move assignment and move constructor below are required for
325 // std::vector<ParamInfo> resize/reallocation and pybind11's type_caster.
! 326 // They manually manage dataPtr refcounts since ParamInfo owns a strong ref.
! 327 ParamInfo& operator=(const ParamInfo& other) {
! 328 if (this != &other) {
! 329 Py_XDECREF(dataPtr);
! 330 inputOutputType = other.inputOutputType;
! 331 paramCType = other.paramCType;
! 332 paramSQLType = other.paramSQLType;
! 333 columnSize = other.columnSize;
! 334 decimalDigits = other.decimalDigits;
! 335 strLenOrInd = other.strLenOrInd;
! 336 isDAE = other.isDAE;
! 337 dataPtr = other.dataPtr;
! 338 utf16Len = other.utf16Len;
! 339 Py_XINCREF(dataPtr);
! 340 }
! 341 return *this;
342 }
! 343 ParamInfo(ParamInfo&& other) noexcept
! 344 : inputOutputType(other.inputOutputType), paramCType(other.paramCType),
! 345 paramSQLType(other.paramSQLType), columnSize(other.columnSize),
! 346 decimalDigits(other.decimalDigits), strLenOrInd(other.strLenOrInd),
! 347 isDAE(other.isDAE), dataPtr(other.dataPtr), utf16Len(other.utf16Len) {
! 348 other.dataPtr = nullptr;
! 349 }
! 350 ParamInfo& operator=(ParamInfo&& other) noexcept {
! 351 if (this != &other) {
! 352 Py_XDECREF(dataPtr);
! 353 inputOutputType = other.inputOutputType;
! 354 paramCType = other.paramCType;
! 355 paramSQLType = other.paramSQLType;
! 356 columnSize = other.columnSize;
! 357 decimalDigits = other.decimalDigits;
! 358 strLenOrInd = other.strLenOrInd;
! 359 isDAE = other.isDAE;
! 360 dataPtr = other.dataPtr;
! 361 utf16Len = other.utf16Len;
! 362 other.dataPtr = nullptr;
! 363 }
! 364 return *this;
365 }
366 };
367 #ifdef __GNUC__
368 #pragma GCC diagnostic popLines 894-903 894 Py_ssize_t time_len = PyUnicode_GET_LENGTH(time_str);
895 info.columnSize = std::max<SQLULEN>(info.columnSize, time_len);
896 // PyList_SetItem (lowercase) decrefs the old slot before stealing the new
897 // reference; safe here because cursor.py already passed a fresh list copy.
! 898 if (PyList_SetItem(params, i, time_str) != 0) {
! 899 throw py::error_already_set();
900 }
901 continue;
902 }Lines 908-918 908 PyObject* as_tuple = PyObject_CallMethod(obj, "as_tuple", NULL);
909 if (!as_tuple) throw py::error_already_set();
910
911 PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent");
! 912 if (!exponent_obj) {
! 913 Py_DECREF(as_tuple);
! 914 throw py::error_already_set();
915 }
916
917 // NaN / Infinity / sNaN: refuse rather than silently writing 0.
918 if (PyUnicode_Check(exponent_obj)) {Lines 922-933 922 "Cannot bind non-finite Decimal (NaN/Infinity) as SQL NUMERIC");
923 }
924
925 PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits");
! 926 if (!digits_obj) {
! 927 Py_DECREF(exponent_obj);
! 928 Py_DECREF(as_tuple);
! 929 throw py::error_already_set();
930 }
931
932 Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj);
933 int exponent = static_cast<int>(PyLong_AsLong(exponent_obj));Lines 931-942 931
932 Py_ssize_t num_digits = PyTuple_GET_SIZE(digits_obj);
933 int exponent = static_cast<int>(PyLong_AsLong(exponent_obj));
934 Py_DECREF(exponent_obj);
! 935 if (exponent == -1 && PyErr_Occurred()) {
! 936 Py_DECREF(digits_obj);
! 937 Py_DECREF(as_tuple);
! 938 throw py::error_already_set();
939 }
940
941 int precision;
942 if (exponent >= 0)Lines 962-973 962 // Use exact Decimal comparison (not double) to avoid boundary misclassification.
963 bool in_money_range = false;
964 int cmp_ge = PyObject_RichCompareBool(obj, PythonObjectCache::smallmoney_min, Py_GE);
965 int cmp_le = PyObject_RichCompareBool(obj, PythonObjectCache::smallmoney_max, Py_LE);
! 966 if (cmp_ge == -1 || cmp_le == -1) {
! 967 Py_DECREF(digits_obj);
! 968 Py_DECREF(as_tuple);
! 969 throw py::error_already_set();
970 }
971 if (cmp_ge == 1 && cmp_le == 1) {
972 in_money_range = true;
973 } else {Lines 972-983 972 in_money_range = true;
973 } else {
974 cmp_ge = PyObject_RichCompareBool(obj, PythonObjectCache::money_min, Py_GE);
975 cmp_le = PyObject_RichCompareBool(obj, PythonObjectCache::money_max, Py_LE);
! 976 if (cmp_ge == -1 || cmp_le == -1) {
! 977 Py_DECREF(digits_obj);
! 978 Py_DECREF(as_tuple);
! 979 throw py::error_already_set();
980 }
981 if (cmp_ge == 1 && cmp_le == 1) {
982 in_money_range = true;
983 }Lines 984-995 984 }
985
986 if (in_money_range) {
987 PyObject* formatted = PyObject_CallMethod(obj, "__format__", "s", "f");
! 988 if (!formatted) {
! 989 Py_DECREF(digits_obj);
! 990 Py_DECREF(as_tuple);
! 991 throw py::error_already_set();
992 }
993 info.paramSQLType = SQL_VARCHAR;
994 info.paramCType = PARAM_C_TYPE_TEXT;
995 info.columnSize = PyUnicode_GET_LENGTH(formatted);Lines 995-1004 995 info.columnSize = PyUnicode_GET_LENGTH(formatted);
996 info.decimalDigits = 0;
997 Py_DECREF(digits_obj);
998 Py_DECREF(as_tuple);
! 999 if (PyList_SetItem(params, i, formatted) != 0) {
! 1000 throw py::error_already_set();
1001 }
1002 continue;
1003 }Lines 1013-1023 1013 info.decimalDigits = nd.scale;
1014 // Store NumericData as a Python object in the param list for the binder.
1015 py::object numeric_obj = py::cast(nd);
1016 PyObject* raw = numeric_obj.release().ptr();
! 1017 if (PyList_SetItem(params, i, raw) != 0) {
! 1018 Py_DECREF(raw);
! 1019 throw py::error_already_set();
1020 }
1021 continue;
1022 }Lines 1030-1040 1030 info.paramSQLType = SQL_GUID;
1031 info.paramCType = SQL_C_GUID;
1032 info.columnSize = 16;
1033 info.decimalDigits = 0;
! 1034 if (PyList_SetItem(params, i, bytes_le) != 0) {
! 1035 Py_DECREF(bytes_le);
! 1036 throw py::error_already_set();
1037 }
1038 continue;
1039 }Lines 1054-1086 1054 PyObject* as_tuple = PyObject_CallMethod(decimal_param, "as_tuple", NULL);
1055 if (!as_tuple) throw py::error_already_set();
1056
1057 PyObject* digits_obj = PyObject_GetAttrString(as_tuple, "digits");
! 1058 if (!digits_obj) {
! 1059 Py_DECREF(as_tuple);
! 1060 throw py::error_already_set();
1061 }
1062 PyObject* sign_obj = PyObject_GetAttrString(as_tuple, "sign");
! 1063 if (!sign_obj) {
! 1064 Py_DECREF(digits_obj);
! 1065 Py_DECREF(as_tuple);
! 1066 throw py::error_already_set();
1067 }
1068 PyObject* exponent_obj = PyObject_GetAttrString(as_tuple, "exponent");
! 1069 if (!exponent_obj) {
! 1070 Py_DECREF(sign_obj);
! 1071 Py_DECREF(digits_obj);
! 1072 Py_DECREF(as_tuple);
! 1073 throw py::error_already_set();
1074 }
1075
1076 int sign_val = static_cast<int>(PyLong_AsLong(sign_obj));
1077 Py_DECREF(sign_obj);
! 1078 if (sign_val == -1 && PyErr_Occurred()) {
! 1079 Py_DECREF(exponent_obj);
! 1080 Py_DECREF(digits_obj);
! 1081 Py_DECREF(as_tuple);
! 1082 throw py::error_already_set();
1083 }
1084
1085 int exponent = 0;
1086 if (PyLong_Check(exponent_obj)) {Lines 1084-1096 1084
1085 int exponent = 0;
1086 if (PyLong_Check(exponent_obj)) {
1087 exponent = static_cast<int>(PyLong_AsLong(exponent_obj));
! 1088 if (exponent == -1 && PyErr_Occurred()) {
! 1089 Py_DECREF(exponent_obj);
! 1090 Py_DECREF(digits_obj);
! 1091 Py_DECREF(as_tuple);
! 1092 throw py::error_already_set();
1093 }
1094 }
1095 Py_DECREF(exponent_obj);Lines 1107-1120 1107 scale = std::min(scale, precision);
1108
1109 PyObject* py_ten = PyLong_FromLong(10);
1110 PyObject* int_val = PyLong_FromLong(0);
! 1111 if (!py_ten || !int_val) {
! 1112 Py_XDECREF(int_val);
! 1113 Py_XDECREF(py_ten);
! 1114 Py_DECREF(digits_obj);
! 1115 Py_DECREF(as_tuple);
! 1116 throw py::error_already_set();
1117 }
1118
1119 const Py_ssize_t digit_count = PyTuple_GET_SIZE(digits_obj);
1120 for (Py_ssize_t i = 0; i < digit_count; ++i) {Lines 1119-1150 1119 const Py_ssize_t digit_count = PyTuple_GET_SIZE(digits_obj);
1120 for (Py_ssize_t i = 0; i < digit_count; ++i) {
1121 PyObject* digit_obj = PyTuple_GET_ITEM(digits_obj, i);
1122 long digit = PyLong_AsLong(digit_obj);
! 1123 if (digit == -1 && PyErr_Occurred()) {
! 1124 Py_DECREF(int_val);
! 1125 Py_DECREF(py_ten);
! 1126 Py_DECREF(digits_obj);
! 1127 Py_DECREF(as_tuple);
! 1128 throw py::error_already_set();
1129 }
1130
1131 PyObject* multiplied = PyNumber_Multiply(int_val, py_ten);
1132 Py_DECREF(int_val);
! 1133 if (!multiplied) {
! 1134 Py_DECREF(py_ten);
! 1135 Py_DECREF(digits_obj);
! 1136 Py_DECREF(as_tuple);
! 1137 throw py::error_already_set();
1138 }
1139
1140 PyObject* py_digit = PyLong_FromLong(digit);
! 1141 if (!py_digit) {
! 1142 Py_DECREF(multiplied);
! 1143 Py_DECREF(py_ten);
! 1144 Py_DECREF(digits_obj);
! 1145 Py_DECREF(as_tuple);
! 1146 throw py::error_already_set();
1147 }
1148
1149 int_val = PyNumber_Add(multiplied, py_digit);
1150 Py_DECREF(multiplied);Lines 1148-1160 1148
1149 int_val = PyNumber_Add(multiplied, py_digit);
1150 Py_DECREF(multiplied);
1151 Py_DECREF(py_digit);
! 1152 if (!int_val) {
! 1153 Py_DECREF(py_ten);
! 1154 Py_DECREF(digits_obj);
! 1155 Py_DECREF(as_tuple);
! 1156 throw py::error_already_set();
1157 }
1158 }
1159
1160 if (exponent > 0) {Lines 1158-1181 1158 }
1159
1160 if (exponent > 0) {
1161 PyObject* multiplier = PyLong_FromLong(1);
! 1162 if (!multiplier) {
! 1163 Py_DECREF(int_val);
! 1164 Py_DECREF(py_ten);
! 1165 Py_DECREF(digits_obj);
! 1166 Py_DECREF(as_tuple);
! 1167 throw py::error_already_set();
1168 }
1169 for (int j = 0; j < exponent; ++j) {
1170 PyObject* next_multiplier = PyNumber_Multiply(multiplier, py_ten);
1171 Py_DECREF(multiplier);
! 1172 if (!next_multiplier) {
! 1173 Py_DECREF(int_val);
! 1174 Py_DECREF(py_ten);
! 1175 Py_DECREF(digits_obj);
! 1176 Py_DECREF(as_tuple);
! 1177 throw py::error_already_set();
1178 }
1179 multiplier = next_multiplier;
1180 }
1181 PyObject* scaled_val = PyNumber_Multiply(int_val, multiplier);Lines 1180-1192 1180 }
1181 PyObject* scaled_val = PyNumber_Multiply(int_val, multiplier);
1182 Py_DECREF(multiplier);
1183 Py_DECREF(int_val);
! 1184 if (!scaled_val) {
! 1185 Py_DECREF(py_ten);
! 1186 Py_DECREF(digits_obj);
! 1187 Py_DECREF(as_tuple);
! 1188 throw py::error_already_set();
1189 }
1190 int_val = scaled_val;
1191 }Lines 1202-1212 1202 if (!val_bytes) throw py::error_already_set();
1203
1204 char* val_buf = nullptr;
1205 Py_ssize_t val_size = 0;
! 1206 if (PyBytes_AsStringAndSize(val_bytes, &val_buf, &val_size) == -1) {
! 1207 Py_DECREF(val_bytes);
! 1208 throw py::error_already_set();
1209 }
1210
1211 NumericData nd;
1212 nd.precision = static_cast<SQLCHAR>(precision);Lines 1469-1478 1469 dataPtr = sqlwcharBuffer->data();
1470 bufferLength = sqlwcharBuffer->size() * sizeof(SQLWCHAR);
1471 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
1472 // Use explicit byte length instead of SQL_NTS so embedded NUL chars
! 1473 // aren't treated as string terminators.
! 1474 *strLenOrIndPtr = static_cast<SQLLEN>(sqlwcharBuffer->size() * sizeof(SQLWCHAR));
1475 }
1476 break;
1477 }
1478 case SQL_C_BIT: {Lines 1611-1619 1611 dataPtr = static_cast<void*>(sqlTimePtr);
1612 break;
1613 }
1614 case SQL_C_SS_TIMESTAMPOFFSET: {
! 1615 py::object datetimeType = PythonObjectCache::get_datetime_class_obj();
1616 if (!py::isinstance(param, datetimeType)) {
1617 ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
1618 }
1619 // Checking if the object has a timezoneLines 2694-2710 2694 }
2695 if (!matchedInfo) {
2696 ThrowStdException("Unrecognized paramToken returned by SQLParamData");
2697 }
! 2698 PyObject* pyObj = matchedInfo->dataPtr;
! 2699 if (!pyObj || pyObj == Py_None) {
2700 putData(nullptr, 0);
2701 continue;
2702 }
! 2703 if (PyUnicode_Check(pyObj)) {
2704 if (matchedInfo->paramCType == SQL_C_WCHAR) {
! 2705 std::u16string utf16 =
! 2706 py::reinterpret_borrow<py::str>(py::handle(pyObj)).cast<std::u16string>();
2707 size_t totalChars = utf16.size();
2708 const SQLWCHAR* dataPtr = reinterpretU16stringAsSqlWChar(utf16);
2709 size_t offset = 0;
2710 size_t chunkChars = DAE_CHUNK_SIZE / sizeof(SQLWCHAR);Lines 2733-2741 2733 py::object encoded = py::reinterpret_borrow<py::object>(py::handle(pyObj))
2734 .attr("encode")(charEncoding, "strict");
2735 encodedStr = encoded.cast<std::string>();
2736 LOG("SQLExecute: DAE SQL_C_CHAR - Encoded with '%s', %zu bytes",
! 2737 charEncoding.c_str(), encodedStr.size());
2738 } catch (const py::error_already_set& e) {
2739 LOG_ERROR("SQLExecute: DAE SQL_C_CHAR - Failed to encode with '%s': %s",
2740 charEncoding.c_str(), e.what());
2741 throw;Lines 2824-2839 2824 SQLRETURN SQLExecute_wrap(const SqlHandlePtr statementHandle,
2825 const std::u16string& query,
2826 py::list params,
2827 py::list is_stmt_prepared,
! 2828 bool use_prepare,
! 2829 const py::dict& encoding_settings) {
2830 if (!statementHandle || !statementHandle->get()) {
2831 return SQL_INVALID_HANDLE;
! 2832 }
! 2833
! 2834 SQLHANDLE hStmt = statementHandle->get();
! 2835
2836 // Configure forward-only / read-only cursor (matches slow path semantics).
2837 if (SQLSetStmtAttr_ptr) {
2838 SQLSetStmtAttr_ptr(hStmt, SQL_ATTR_CURSOR_TYPE,
2839 (SQLPOINTER)SQL_CURSOR_FORWARD_ONLY, 0);Lines 2850-2876 2850 // encoding when ctype == 1 (real ODBC SQL_CHAR). Otherwise the user's
2851 // "encoding" value is meant for the wide-char path and we leave it alone.
2852 std::string charEncoding = "utf-8";
2853 if (encoding_settings.contains("ctype") && encoding_settings.contains("encoding")) {
! 2854 int ctype = encoding_settings["ctype"].cast<int>();
! 2855 if (ctype == SQL_C_CHAR /* real ODBC value: 1 */) {
! 2856 charEncoding = encoding_settings["encoding"].cast<std::string>();
! 2857 }
! 2858 }
! 2859
! 2860 // The cursor.py caller always passes a fresh `list(actual_params)` so this
! 2861 // function is free to mutate slots in place. Even so, every site below uses
! 2862 // PyList_SetItem (which decrefs the old slot before stealing the new ref),
! 2863 // so the function is safe regardless of who owns the list.
! 2864
! 2865 // Run DetectParamTypes BEFORE SQLPrepare so that type-detection errors
! 2866 // (unsupported type, NaN Decimal, precision overflow) don't leave the
! 2867 // cursor in a half-prepared state.
! 2868 std::vector<ParamInfo> paramInfos = DetectParamTypes(params.ptr());
! 2869
! 2870 RETCODE rc;
! 2871 bool already_prepared = is_stmt_prepared[0].cast<bool>();
! 2872
2873 // Honor use_prepare flag (matching slow path behavior):
2874 // - use_prepare=true: prepare now (or reuse if same SQL already prepared)
2875 // - use_prepare=false + already prepared: reuse existing plan
2876 // - use_prepare=false + not prepared: error (cannot execute unprepared)Lines 2899-2908 2899 }
2900
2901 // DAE (Data-At-Execution) loop: when BindParameters marks a param as DAE
2902 // (large str/bytes/binary), SQLExecute returns SQL_NEED_DATA. We must
! 2903 // stream the data via SQLParamData/SQLPutData before execution completes.
! 2904 // GIL is released around each ODBC call to match slow-path concurrency.
2905 if (rc == SQL_NEED_DATA) {
2906 SQLPOINTER paramToken = nullptr;
2907 while (true) {
2908 {Lines 2930-2940 2930
2931 if (PyUnicode_Check(pyObj)) {
2932 if (matchedInfo->paramCType == SQL_C_WCHAR) {
2933 std::u16string u16 =
! 2934 py::reinterpret_borrow<py::str>(py::handle(pyObj)).cast<std::u16string>();
! 2935 const SQLWCHAR* dataPtr = reinterpretU16stringAsSqlWChar(u16);
! 2936 size_t totalChars = u16.size();
2937 size_t chunkChars = DAE_CHUNK_SIZE / sizeof(SQLWCHAR);
2938 for (size_t offset = 0; offset < totalChars; offset += chunkChars) {
2939 size_t len = std::min(chunkChars, totalChars - offset);
2940 {Lines 2942-2958 2942 rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset),
2943 static_cast<SQLLEN>(len * sizeof(SQLWCHAR)));
2944 }
2945 if (!SQL_SUCCEEDED(rc)) return rc;
! 2946 }
! 2947 } else if (matchedInfo->paramCType == SQL_C_CHAR) {
! 2948 std::string encodedStr;
! 2949 py::object encoded = py::reinterpret_borrow<py::object>(py::handle(pyObj))
! 2950 .attr("encode")(charEncoding, "strict");
2951 encodedStr = encoded.cast<std::string>();
2952 const char* dataPtr = encodedStr.data();
2953 size_t totalBytes = encodedStr.size();
! 2954 for (size_t offset = 0; offset < totalBytes; offset += DAE_CHUNK_SIZE) {
2955 size_t len = std::min(static_cast<size_t>(DAE_CHUNK_SIZE),
2956 totalBytes - offset);
2957 {
2958 py::gil_scoped_release release;Lines 2956-2964 2956 totalBytes - offset);
2957 {
2958 py::gil_scoped_release release;
2959 rc = SQLPutData_ptr(hStmt, (SQLPOINTER)(dataPtr + offset),
! 2960 static_cast<SQLLEN>(len));
2961 }
2962 if (!SQL_SUCCEEDED(rc)) return rc;
2963 }
2964 } else {Lines 2976-2984 2976 dataPtr = bytesStorage.data();
2977 totalBytes = bytesStorage.size();
2978 } else {
2979 // bytearray is mutable — copy to stable buffer before streaming
! 2980 bytesStorage.assign(PyByteArray_AS_STRING(pyObj),
2981 static_cast<size_t>(PyByteArray_GET_SIZE(pyObj)));
2982 dataPtr = bytesStorage.data();
2983 totalBytes = bytesStorage.size();
2984 }Lines 2992-3003 2992 static_cast<SQLLEN>(len));
2993 }
2994 if (!SQL_SUCCEEDED(rc)) return rc;
2995 }
! 2996 } else {
! 2997 ThrowStdException("SQLExecuteFast: DAE only supported for str or bytes");
! 2998 }
! 2999 }
3000 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;
3001 }
3002
3003 if (!SQL_SUCCEEDED(rc) && rc != SQL_NO_DATA) return rc;Lines 3005-3013 3005 // Preserve the execute return code (e.g. SQL_SUCCESS_WITH_INFO) — don't
3006 // let the SQLFreeStmt return value clobber what the caller needs to see.
3007 SQLRETURN exec_rc = rc;
3008 SQLFreeStmt_ptr(hStmt, SQL_RESET_PARAMS);
! 3009 return exec_rc;
3010 }
3011
3012 SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list& columnwise_params,
3013 std::vector<ParamInfo>& paramInfos, size_t paramSetSize,Lines 3432-3440 3432 DateTimeOffset* dtoArray =
3433 AllocateParamBufferArray<DateTimeOffset>(tempBuffers, paramSetSize);
3434 strLenOrIndArray = AllocateParamBufferArray<SQLLEN>(tempBuffers, paramSetSize);
3435
! 3436 py::object datetimeType = PythonObjectCache::get_datetime_class_obj();
3437
3438 for (size_t i = 0; i < paramSetSize; ++i) {
3439 const py::handle& param = columnValues[i];Lines 3547-3555 3547
3548 // Get cached UUID class from module-level helper
3549 // This avoids static object destruction issues during
3550 // Python finalization
! 3551 py::object uuid_class = PythonObjectCache::get_uuid_class_obj();
3552 // Get cached UUID class
3553
3554 for (size_t i = 0; i < paramSetSize; ++i) {
3555 const py::handle& element = columnValues[i];Lines 4636-4644 4636 int microseconds = dtoValue.fraction / 1000;
4637 py::object datetime_module = py::module_::import("datetime");
4638 py::object tzinfo = datetime_module.attr("timezone")(
4639 datetime_module.attr("timedelta")(py::arg("minutes") = totalMinutes));
! 4640 py::object py_dt = PythonObjectCache::get_datetime_class_obj()(
4641 dtoValue.year, dtoValue.month, dtoValue.day, dtoValue.hour, dtoValue.minute,
4642 dtoValue.second, microseconds, tzinfo);
4643 row.append(py_dt);
4644 } else {📋 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: 74.1%
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>
| 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) |
…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)); | ||
| } |
Comment on lines
+2976
to
+2980
| } else { | ||
| // bytearray: use raw buffer access | ||
| dataPtr = PyByteArray_AS_STRING(pyObj); | ||
| totalBytes = static_cast<size_t>(PyByteArray_GET_SIZE(pyObj)); | ||
| } |
| info.columnSize = std::max<SQLULEN>(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); |
| info.decimalDigits = 0; | ||
| Py_DECREF(digits_obj); | ||
| Py_DECREF(as_tuple); | ||
| PyList_SetItem(params, i, formatted); |
Comment on lines
+1022
to
+1024
| // 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()); |
| info.paramCType = SQL_C_GUID; | ||
| info.columnSize = 16; | ||
| info.decimalDigits = 0; | ||
| PyList_SetItem(params, i, bytes_le); |
…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>
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