Repository navigation
PERF: Reuse cursor-scoped state for row fetching - #858
Open
Jahnvi Thakkar (jahnvi480) wants to merge 10 commits into
Open
Jahnvi Thakkar (jahnvi480) wants to merge 10 commits into
Jahnvi Thakkar (jahnvi480) wants to merge 10 commits into
Conversation
Recover the exact cumulative Source05 native-fetch implementation and its regression coverage against frozen main. Preserve watcher invalidation, ownership, fallback and failure controls without later redesigns. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR Performance Report✅ Performance improved2 database tasks consistently improved across 2 measured environments. No consistent slowdowns were detected. 2 IMPROVEMENTS 0 SLOWDOWNS 2/2 ENVIRONMENTS Signal fingerprint
Coverage: 2 of 2 environments completed. Advisory result; does not block merging. Measured timings
Performance diagnosticsPhase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed. Unix / SQL Server 2022Row-by-row fetching: py::fetchone::cpp_call -4.793 ms; ddbc::FetchOne_wrap -3.963 ms; py::fetchone::row_wrap -0.611 ms. Call changes: ddbc::FetchOne_wrap (1001 -> 1 calls); ddbc::FetchSingleRow (added, removed, or intermittent); ddbc::FetchSingleRow::SQL_UNBIND (added, removed, or intermittent). Unix / SQL Server 2025Row-by-row fetching: py::fetchone::cpp_call -5.119 ms; ddbc::FetchOne_wrap -4.174 ms; py::fetchone::row_wrap -0.611 ms. Call changes: ddbc::FetchOne_wrap (1001 -> 1 calls); ddbc::FetchSingleRow (added, removed, or intermittent); ddbc::FetchSingleRow::SQL_UNBIND (added, removed, or intermittent). All database tasks and timingsUnix / SQL Server 2022
Unix / SQL Server 2025
Build and measurement detailsPR head:
A consistent change requires more than 20% median paired movement, at least 1 ms between the median runtimes, and at least 80% of pairs exceeding the relative threshold in the same direction. A slowdown without enough pair agreement is reported as inconsistent. The displayed change is the median of paired before-and-after ratios. It is not recalculated from the two displayed median runtimes. Both revisions use profiling-enabled builds on the same agent and database, with alternating order and discarded warmups. Results are diagnostic and do not represent production-wheel latency. Raw samples and logs are attached to the ADO run as |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Native ownership and watcher-based dispatch need human review, with fallback compatibility fixes, cross-platform validation, and performance qualification still outstanding.
2 open issues
What changed in this PR
This PR reduces repeated single-row fetch overhead by reusing cursor-scoped native state while retaining Python fallback paths.
Changes:
- Adds watcher-based native dispatch and garbage-collector-visible ownership for eligible single-row operations.
- Shares the native single-row pipeline, tracks complete metadata, and reuses successful column unbinding.
- Adds parity, mutation, lifetime, operation-count, and isolated failure-control tests.
| File | Description |
|---|---|
| tests/test_single_row_pipeline.py | Tests single-row parity, ownership, and operation counts. |
| tests/test_native_fetch_dispatch.py | Tests dispatch, mutation, lifetime, and failure recovery. |
| mssql_python/pybind/row_factory.hpp | Adds single-row construction and reusable attribute names. |
| mssql_python/pybind/result_metadata.hpp | Distinguishes complete metadata from partial prefixes. |
| mssql_python/pybind/native_fetch.hpp | Implements native state, watchers, dispatch, and fallback. |
| mssql_python/pybind/ddbc_bindings.h | Tracks reusable column-unbinding state. |
| mssql_python/pybind/ddbc_bindings.cpp | Shares single-row fetching and registers native bindings. |
| mssql_python/pybind/CMakeLists.txt | Gates fault controls behind profiling-enabled builds. |
| mssql_python/cursor.py | Adds invalidation, completion helpers, and dispatch installation. |
| mssql_python/_native_fetch.py | Defines eligibility policy and installation rollback. |
Review effort: Balanced
Preserve fallback semantics for dict subclasses and keyword binding, accept CPython 3.15's ordinary dictionary descriptor, and initialize stable policy dependencies before capture. Restore logging state between tests and consolidate duplicate fetch coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/_native_fetch.pyLines 99-107 99 return None
100 from mssql_python.connection import Connection
101
102 if type(connection) is not Connection:
! 103 return None
104 descriptor = vars(Connection).get("__dict__")
105 if (
106 Connection.__getattribute__ is not object.__getattribute__
107 or type(descriptor) is not GetSetDescriptorTypeLines 108-116 108 # CPython 3.15 shares the ordinary instance-dict descriptor on object.
109 or descriptor.__objclass__ not in (Connection, object)
110 or descriptor.__name__ != "__dict__"
111 ):
! 112 return None
113 connection_data = object.__getattribute__(connection, "__dict__")
114 ddbc_bindings._watch_fetch_dependencies(
115 self.observer,
116 (data, connection_data),Lines 150-158 150 for name, value in original.items()
151 ):
152 return None
153 if self.logger.is_debug_enabled:
! 154 return None
155 if (
156 self.row_type.__new__ is not object.__new__
157 or self.row_type.__setattr__ is not object.__setattr__
158 or any(Lines 160-168 160 for base in Connection.__mro__
161 for name in ("_decoding_generation", "_converters_generation")
162 )
163 ):
! 164 return None
165 if any(name in data for name in vars(self.cursor_type)):
166 return None
167 for cached, generation in (
168 ("_cached_decoding_generation", "_decoding_generation"),Lines 171-179 171 if (
172 type(data.get(cached)) is not int
173 or type(connection_data.get(generation)) is not int
174 ):
! 175 return None
176 if (
177 data.get("closed") is not False
178 or data.get("_has_result_set") is not True
179 or data.get("_skip_increment_for_next_fetch") is not FalseLines 241-251 241
242
243 class _InstallRollbackError(RuntimeError):
244 def __init__(self, installation_error, cleanup_errors):
! 245 self.installation_error = installation_error
! 246 self.cleanup_errors = tuple(cleanup_errors)
! 247 super().__init__(
248 "Native fetch installation and rollback both failed",
249 installation_error,
250 self.cleanup_errors,
251 )Lines 253-261 253
254 def install(cursor_type, row_type, namespace):
255 observer = ddbc_bindings._fetch_observer
256 if observer is None or not isinstance(getattr(row_type, "_fast_create", None), FunctionType):
! 257 return None
258 methods = ("fetchone", "fetchmany", "fetchval", "__next__")
259 fallbacks = tuple(getattr(cursor_type, name) for name in methods)
260 entries = ddbc_bindings._native_fetch_methods
261 originals = {name: vars(cursor_type)[name] for name in methods}Lines 278-302 278 ("_next_row_index", "_rownumber", "rowcount"),
279 context,
280 )
281 if any(vars(cursor_type).get(name) is not entry for name, entry in zip(methods, entries)):
! 282 raise RuntimeError("Cursor methods changed during native fetch installation")
283 activate(runtime)
284 return runtime
! 285 except BaseException as installation_error:
286 # Unpublished entries can only forward to originals until activation.
! 287 cleanup_errors = []
! 288 for name, entry in zip(methods, entries):
! 289 try:
! 290 if vars(cursor_type).get(name) is entry:
! 291 setattr(cursor_type, name, originals[name])
! 292 except BaseException as cleanup_error:
! 293 cleanup_errors.append((name, cleanup_error))
! 294 survivors = tuple(
295 name for name, entry in zip(methods, entries) if vars(cursor_type).get(name) is entry
296 )
! 297 if survivors and not cleanup_errors:
! 298 cleanup_errors.append(
299 (
300 "descriptors",
301 RuntimeError("Native fetch descriptors survived rollback", survivors),
302 )Lines 302-314 302 )
303 )
304 # Retained descriptors must retain their forwarding functions. A partial
305 # rollback is terminal, not a context that a later install may reuse.
! 306 terminalize = disable if cleanup_errors else abort
! 307 try:
! 308 terminalize()
! 309 except BaseException as cleanup_error:
! 310 cleanup_errors.append(("context", cleanup_error))
! 311 if cleanup_errors:
! 312 raise _InstallRollbackError(installation_error, cleanup_errors) from installation_error
! 313 raisemssql_python/cursor.pyLines 2898-2912 2898 def _complete_fetchone(self, row_data, ret):
2899 """Complete an already-consumed fetch, including mutation/error fallback."""
2900 check_error(_SQL_HANDLE_STMT, self.hstmt, ret)
2901 if ret == _SQL_NO_DATA:
! 2902 if self._next_row_index == 0 and self.description is not None:
! 2903 self.rowcount = 0
! 2904 return None
2905
2906 if self._skip_increment_for_next_fetch:
! 2907 self._skip_increment_for_next_fetch = False
! 2908 self._next_row_index += 1
2909 else:
2910 self._increment_rownumber()
2911 self.rowcount = self._next_row_index
2912 return self._wrap_fetchone(row_data)Lines 2917-2925 2917 if not converter_map and not self._uuid_str_indices:
2918 return Row._fast_create(
2919 row_data, column_map, self, column_map_lower, self._cached_result_columns
2920 )
! 2921 return Row(
2922 row_data,
2923 column_map,
2924 cursor=self,
2925 converter_map=converter_map,mssql_python/pybind/ddbc_bindings.cppLines 5105-5113 5105 if (fetchSize == 1 && lobColumns.empty()) {
5106 py::list row;
5107 ret = FetchSingleRow(StatementHandle, row, charEncoding, wcharEncoding, charCtype, messages);
5108 if (SQL_SUCCEEDED(ret))
! 5109 rows.append(row);
5110 return ret;
5111 }
5112
5113 // Initialized to 0 for LOB path counter; overwritten by ODBC in non-LOB path;mssql_python/pybind/native_fetch.hppLines 34-42 34 PyObject* value) {
35 if (event != PyDict_EVENT_MODIFIED || dict != internalDict ||
36 (key != internalKey &&
37 (!key || !internalKey || !PyUnicode_CheckExact(key) ||
! 38 PyUnicode_Compare(key, internalKey) != 0)) ||
39 value != internalValue)
40 mutationEpoch.fetch_add(1, std::memory_order_relaxed);
41 return 0;
42 }Lines 271-279 271 while (PyDict_Next(dict, &position, &key, &value)) {
272 if (PyUnicode_CheckExact(key) && PyUnicode_GET_LENGTH(key) == length) {
273 const int comparison = PyUnicode_CompareWithASCIIString(key, name);
274 if (PyErr_Occurred())
! 275 throw py::error_already_set();
276 if (comparison == 0)
277 return value;
278 }
279 }Lines 297-305 297 } else if (PyUnicode_GET_LENGTH(key) ==
298 static_cast<Py_ssize_t>(sizeof("_native_fetch_state") - 1)) {
299 const int comparison = PyUnicode_CompareWithASCIIString(key, "_native_fetch_state");
300 if (PyErr_Occurred())
! 301 throw py::error_already_set();
302 if (comparison == 0)
303 result.value = value;
304 }
305 }Lines 307-315 307 }
308
309 inline bool owned_by(State* state, Context* context, PyObject* receiver) {
310 if (state->context != reinterpret_cast<PyObject*>(context) || !state->owner)
! 311 return false;
312 #if PY_VERSION_HEX >= 0x030D0000
313 PyObject* owner = nullptr;
314 const int result = PyWeakref_GetRef(state->owner, &owner);
315 if (result < 0)Lines 319-327 319 #else
320 // The GIL and the call's strong receiver reference cover this borrowed read.
321 PyObject* owner = PyWeakref_GetObject(state->owner);
322 if (!owner)
! 323 throw py::error_already_set();
324 return owner == receiver;
325 #endif
326 }Lines 339-350 339 reinterpret_cast<void*>(PyObject_GenericSetAttr))
340 return false;
341 auto attributes = steal(PyType_GetDict(Py_TYPE(receiver)));
342 if (!attributes)
! 343 throw py::error_already_set();
344 if (exact_item(attributes.ptr(), "__dict__") != context->dictDescriptor ||
345 exact_item(attributes.ptr(), "_native_fetch_state"))
! 346 return false;
347 auto* handle = exact_item(dict, "hstmt");
348 auto* index = exact_item(dict, "_next_row_index");
349 auto* messages = exact_item(dict, "messages");
350 return exact_item(dict, "closed") == Py_False && handle && handle != Py_None &&Lines 407-415 407 #endif
408 weakOwner = PyWeakref_NewRef(receiver, nullptr);
409 auto owner = steal(weakOwner);
410 if (!owner)
! 411 throw py::error_already_set();
412 PyObject* allocated;
413 #ifdef MSSQL_NATIVE_FETCH_TEST_FAULTS
414 if (test_fault(context, 2))
415 allocated = PyErr_NoMemory();Lines 418-426 418 allocated = PyType_GenericAlloc(
419 reinterpret_cast<PyTypeObject*>(context->stateType), 0);
420 auto result = steal(allocated);
421 if (!result)
! 422 throw py::error_already_set();
423 auto* state = reinterpret_cast<State*>(result.ptr());
424 #ifdef MSSQL_NATIVE_FETCH_TEST_FAULTS
425 state->testContext = context;
426 ++context->faults.statesAllocated;Lines 441-453 441 // Allocation can run GC callbacks. Revalidate before a callback-free write,
442 // retaining the old value so publication cannot release its last reference.
443 auto now = instance_dict(receiver);
444 if (!now)
! 445 throw py::error_already_set();
446 entry = state_entry(dict.ptr());
447 if (!now.is(dict) || entry.value != old.ptr() ||
448 !storage_admissible(context, receiver, dict.ptr(), entry))
! 449 return {};
450 int published;
451 #ifdef MSSQL_NATIVE_FETCH_TEST_FAULTS
452 if (test_fault(context, 4)) {
453 PyErr_NoMemory();Lines 455-463 455 } else
456 #endif
457 published = PyDict_SetItem(dict.ptr(), context->stateKey, result.ptr());
458 if (published < 0)
! 459 throw py::error_already_set();
460 #ifdef MSSQL_NATIVE_FETCH_TEST_FAULTS
461 ++context->faults.publications;
462 #endif
463 return result;Lines 489-497 489 auto plan = std::make_shared<Plan>(std::move(dict), epoch);
490 py::object values = rt[2](py::handle(self), plan->cursorDict);
491 if (!values.is_none()) {
492 if (!PyTuple_CheckExact(values.ptr()) || PyTuple_GET_SIZE(values.ptr()) != InputCount)
! 493 throw py::type_error("Invalid native fetch plan");
494 plan->inputs = std::move(values);
495 if (!PyDict_CheckExact(plan->at(CursorDict).ptr()) ||
496 !PyDict_CheckExact(plan->at(ConnectionDict).ptr()) ||
497 !PyDict_CheckExact(plan->at(ColumnMap).ptr()) ||Lines 503-516 503 !PyType_Check(plan->at(ConnectionType).ptr()) ||
504 !PyLong_CheckExact(plan->at(NextIndex).ptr()) ||
505 !PyLong_CheckExact(plan->at(ArraySize).ptr()) ||
506 !PyCallable_Check(plan->at(Factory).ptr()))
! 507 throw py::type_error("Invalid native fetch plan fields");
508 // current() relies on this watch, not on the Python policy registering it.
509 auto* observer = static_cast<Observer*>(
510 PyCapsule_GetPointer(rt[0].ptr(), "mssql_python.native_fetch.Observer"));
511 if (!observer || PyDict_Watch(observer->dict, plan->cursorDict.ptr()) < 0)
! 512 throw py::error_already_set();
513 plan->handle = plan->at(Handle).cast<SqlHandlePtr>();
514 plan->charEncoding = plan->at(CharEncoding).cast<std::string>();
515 plan->wideEncoding = plan->at(WideEncoding).cast<std::string>();
516 plan->charCtype = EffectiveCharCtypeForFetch(Lines 541-549 541 return false;
542 auto dict = instance_dict(self);
543 auto connectionDict = instance_dict(plan.at(Connection).ptr());
544 if (!dict || !connectionDict)
! 545 throw py::error_already_set();
546 // fetch() checks the key association before use. The native dictionary watch
547 // invalidates this epoch on a later state-key change; replacement is checked here.
548 return dict.is(plan.cursorDict) && connectionDict.is(plan.at(ConnectionDict)) &&
549 state->dict == dict.ptr() && owned_by(state, context, self) &&Lines 564-572 564 internalDict = plan.cursorDict.ptr();
565 internalKey = name.ptr();
566 internalValue = value.ptr();
567 if (PyDict_SetItem(plan.cursorDict.ptr(), name.ptr(), value.ptr()) < 0)
! 568 throw py::error_already_set();
569 }
570
571 inline py::object finish_python(PyObject* self, Mode mode, const py::list& values,
572 SQLRETURN ret, bool positionCommitted) {Lines 576-584 576 py::list rows;
577 if (SQL_SUCCEEDED(ret))
578 rows.append(values);
579 if (positionCommitted)
! 580 return cursor.attr("_wrap_fetchmany")(rows);
581 return cursor.attr("_complete_fetchmany")(rows, ret);
582 }
583 py::object row = positionCommitted ? cursor.attr("_wrap_fetchone")(values)
584 : cursor.attr("_complete_fetchone")(values, ret);Lines 584-592 584 : cursor.attr("_complete_fetchone")(values, ret);
585 if (mode == Mode::Value && !row.is_none())
586 return row[py::int_(0)];
587 if (mode == Mode::Next && row.is_none())
! 588 throw py::stop_iteration();
589 return row;
590 }
591
592 inline bool conversion_ineligible(py::handle data) {Lines 609-617 609 else if (length == static_cast<Py_ssize_t>(sizeof("_uuid_str_indices") - 1) &&
610 PyUnicode_CompareWithASCIIString(key, "_uuid_str_indices") == 0)
611 uuidIndices = value;
612 if (PyErr_Occurred())
! 613 throw py::error_already_set();
614 }
615 return !converters || !PyTuple_CheckExact(converters) ||
616 PyTuple_GET_SIZE(converters) != 0 || uuidIndices != Py_None;
617 }Lines 626-634 626 if (!PyUnicode_CheckExact(key))
627 return false;
628 const int comparison = PyUnicode_CompareWithASCIIString(key, "size");
629 if (PyErr_Occurred())
! 630 throw py::error_already_set();
631 return comparison == 0;
632 }
633
634 inline bool explicit_nonunit_size(PyObject* const* args, Py_ssize_t nargs,Lines 639-647 639 return false;
640 int overflow = 0;
641 const long size = PyLong_AsLongAndOverflow(args[0], &overflow);
642 if (PyErr_Occurred())
! 643 throw py::error_already_set();
644 return overflow || size != 1;
645 }
646
647 inline bool single_size(const Plan& plan, PyObject* const* args, Py_ssize_t nargs,Lines 654-662 654 return false;
655 int overflow = 0;
656 const long value = PyLong_AsLongAndOverflow(size, &overflow);
657 if (PyErr_Occurred())
! 658 throw py::error_already_set();
659 return !overflow && value == 1;
660 }
661
662 inline py::object fetch(PyObject* self, Context* context, Mode mode,Lines 676-684 676 return fallback(self, rt, mode, args, nargs, keywords);
677
678 py::dict dict = instance_dict(object);
679 if (!dict)
! 680 throw py::error_already_set();
681 bool needsRevalidation;
682 auto stateOwner = acquire_state(context, self, dict, needsRevalidation);
683 auto* state = reinterpret_cast<State*>(stateOwner.ptr());
684 auto admittedDict = needsRevalidation ? instance_dict(self) : dict;Lines 682-693 682 auto stateOwner = acquire_state(context, self, dict, needsRevalidation);
683 auto* state = reinterpret_cast<State*>(stateOwner.ptr());
684 auto admittedDict = needsRevalidation ? instance_dict(self) : dict;
685 if (!admittedDict)
! 686 throw py::error_already_set();
687 if (!admittedDict.is(dict) ||
688 (needsRevalidation && state && !associated(state, context, self, dict.ptr())))
! 689 return fallback(self, rt, mode, args, nargs, keywords);
690 const uint64_t epoch = mutationEpoch.load(std::memory_order_relaxed);
691 auto plan = state && state->plan ? *state->plan : nullptr;
692 bool stale = !plan || plan->epoch != epoch || !plan->cursorDict.is(dict);
693 if (!stale && !plan->inputs.is_none()) {Lines 699-707 699 if (stale) {
700 plan = prepare(self, rt, dict, epoch);
701 auto preparedDict = instance_dict(self);
702 if (!preparedDict)
! 703 throw py::error_already_set();
704 if (state && state->plan && preparedDict.is(dict) &&
705 associated(state, context, self, dict.ptr())) {
706 auto old = std::move(*state->plan);
707 *state->plan = plan;Lines 716-727 716 return fallback(self, rt, mode, args, nargs, keywords);
717 auto metadata = plan->handle->resultMetadata.snapshot();
718 if (metadata.generation != plan->metadata.generation ||
719 metadata.metadata != plan->metadata.metadata)
! 720 return fallback(self, rt, mode, args, nargs, keywords);
721 }
722 if (mode == Mode::Value && PyList_GET_SIZE(plan->at(Description).ptr()) == 0)
! 723 return fallback(self, rt, mode, args, nargs, keywords);
724
725 PERF_TIMER("NativeCursor::fetch");
726 py::list values;
727 SQLRETURN ret = FetchSingleRow(plan->handle, values, plan->charEncoding,Lines 738-750 738 throw py::stop_iteration();
739 return py::none();
740 }
741 if (plan->nextIndex == std::numeric_limits<Py_ssize_t>::max())
! 742 return finish_python(self, mode, values, ret, false);
743 py::int_ next(plan->nextIndex + 1);
744 py::int_ rowNumber(plan->nextIndex);
745 if (!current(self, context, state, *plan, rt))
! 746 return finish_python(self, mode, values, ret, false);
747 auto counterNames = py::reinterpret_borrow<py::tuple>(rt[5]);
748 set_counter(*plan, counterNames[0], next);
749 set_counter(*plan, counterNames[1], rowNumber);
750 set_counter(*plan, counterNames[2], next);Lines 749-757 749 set_counter(*plan, counterNames[1], rowNumber);
750 set_counter(*plan, counterNames[2], next);
751 ++plan->nextIndex;
752 if (!current(self, context, state, *plan, rt))
! 753 return finish_python(self, mode, values, ret, true);
754
755 if (mode == Mode::Value) {
756 // Decode every column, not just column zero. Eligible plans have no
757 // converters, UUID processing or customized Row/fetchone behavior.Lines 783-791 783 PyObject* keywords) {
784 try {
785 auto* context = reinterpret_cast<Context*>(owner);
786 if (!context->functions)
! 787 throw std::runtime_error("Native fetch context is not initialized");
788 if (context->runtime && count > 0 &&
789 PyObject_TypeCheck(args[0], reinterpret_cast<PyTypeObject*>(context->cursorType)) &&
790 (mode == Mode::Many
791 ? valid_many_arguments(count - 1, keywords)Lines 814-845 814 auto* context = reinterpret_cast<Context*>(owner);
815 if (context->disabled)
816 throw std::runtime_error("Native fetch installation is disabled after incomplete rollback");
817 if (context->cursorType || context->active)
! 818 throw std::runtime_error("Native fetch context is already installed");
819 PyObject* cursorType;
820 PyObject* functions;
821 if (!PyArg_ParseTuple(args, "OO", &cursorType, &functions))
! 822 return nullptr;
823 if (!PyType_CheckExact(cursorType) || !PyTuple_CheckExact(functions) ||
824 PyTuple_GET_SIZE(functions) != 4)
! 825 throw py::type_error("Invalid native fetch installation");
826 auto bases = steal(PyObject_GetAttrString(cursorType, "__bases__"));
827 if (!bases)
! 828 throw py::error_already_set();
829 if (!PyTuple_CheckExact(bases.ptr()) || PyTuple_GET_SIZE(bases.ptr()) != 1 ||
830 PyTuple_GET_ITEM(bases.ptr(), 0) != reinterpret_cast<PyObject*>(&PyBaseObject_Type))
! 831 throw py::type_error("Native fetch requires a plain Python cursor");
832 for (Py_ssize_t i = 0; i < 4; ++i) {
833 if (!PyFunction_Check(PyTuple_GET_ITEM(functions, i)))
! 834 throw py::type_error("Native fetch fallbacks must be Python functions");
835 }
836 auto attributes = steal(PyType_GetDict(reinterpret_cast<PyTypeObject*>(cursorType)));
837 if (!attributes)
! 838 throw py::error_already_set();
839 auto* descriptor = exact_item(attributes.ptr(), "__dict__");
840 if (!descriptor || Py_TYPE(descriptor) != &PyGetSetDescr_Type)
! 841 throw py::type_error("Native fetch requires an ordinary cursor dictionary");
842 context->cursorType = Py_NewRef(cursorType);
843 context->functions = Py_NewRef(functions);
844 context->dictDescriptor = Py_NewRef(descriptor);
845 Py_RETURN_NONE;Lines 866-876 866 for (Py_ssize_t field = 4; field <= 5; ++field) {
867 auto* names = PyTuple_GET_ITEM(runtime, field);
868 const Py_ssize_t count = field == 4 ? 5 : 3;
869 if (!PyTuple_CheckExact(names) || PyTuple_GET_SIZE(names) != count) {
! 870 PyErr_SetString(PyExc_TypeError, "Invalid native fetch field names");
! 871 return nullptr;
! 872 }
873 for (Py_ssize_t i = 0; i < count; ++i) {
874 auto* name = PyTuple_GET_ITEM(names, i);
875 if (!PyUnicode_CheckExact(name) ||
876 PyUnicode_CompareWithASCIIString(Lines 874-885 874 auto* name = PyTuple_GET_ITEM(names, i);
875 if (!PyUnicode_CheckExact(name) ||
876 PyUnicode_CompareWithASCIIString(
877 name, field == 4 ? rowNames[i] : counterNames[i]) != 0) {
! 878 if (!PyErr_Occurred())
! 879 PyErr_SetString(PyExc_TypeError, "Invalid native fetch field name");
! 880 return nullptr;
! 881 }
882 }
883 }
884 context->runtime = Py_NewRef(runtime);
885 Py_RETURN_NONE;Lines 887-897 887
888 inline PyObject* abort_install(PyObject* owner, PyObject*) {
889 auto* context = reinterpret_cast<Context*>(owner);
890 if (context->disabled || context->runtime || context->active) {
! 891 PyErr_SetString(PyExc_RuntimeError, "Cannot abort an active native fetch context");
! 892 return nullptr;
! 893 }
894 auto* cursorType = context->cursorType;
895 auto* functions = context->functions;
896 auto* descriptor = context->dictDescriptor;
897 context->cursorType = context->functions = context->dictDescriptor = nullptr;Lines 903-913 903
904 inline PyObject* disable_install(PyObject* owner, PyObject*) {
905 auto* context = reinterpret_cast<Context*>(owner);
906 if (context->runtime || context->active) {
! 907 PyErr_SetString(PyExc_RuntimeError, "Cannot disable an active native fetch context");
! 908 return nullptr;
! 909 }
910 // Keep every forwarding owner: even descriptors restored on the class may
911 // have been saved elsewhere. Unpublished entries can never create State.
912 context->disabled = true;
913 Py_RETURN_NONE;Lines 984-992 984 auto* context = reinterpret_cast<Context*>(owner);
985 if (reinterpret_cast<PyObject*>(Py_TYPE(receiver)) == context->cursorType) {
986 auto dict = instance_dict(receiver);
987 if (!dict)
! 988 throw py::error_already_set();
989 auto* value = state_entry(dict.ptr()).value;
990 if (value && reinterpret_cast<PyObject*>(Py_TYPE(value)) == context->stateType) {
991 auto stateOwner = py::reinterpret_borrow<py::object>(value);
992 auto* state = reinterpret_cast<State*>(value);Lines 996-1006 996 }
997 }
998 Py_RETURN_NONE;
999 } catch (...) {
! 1000 py::detail::try_translate_exceptions();
! 1001 return nullptr;
! 1002 }
1003 }
1004
1005 inline void bind(py::module_& m) {
1006 #ifdef _MSC_VERLines 1041-1049 1041 Py_TPFLAGS_IMMUTABLETYPE,
1042 stateSlots};
1043 auto stateType = steal(PyType_FromSpec(&stateSpec));
1044 if (!stateType)
! 1045 throw py::error_already_set();
1046 static PyType_Slot contextSlots[] = {
1047 {Py_tp_new, reinterpret_cast<void*>(internal_new)},
1048 {Py_tp_dealloc, reinterpret_cast<void*>(dealloc<Context, context_clear>)},
1049 {Py_tp_traverse, reinterpret_cast<void*>(context_traverse)},Lines 1055-1072 1055 Py_TPFLAGS_IMMUTABLETYPE,
1056 contextSlots};
1057 auto contextType = steal(PyType_FromSpec(&contextSpec));
1058 if (!contextType)
! 1059 throw py::error_already_set();
1060 auto contextOwner = steal(PyType_GenericAlloc(
1061 reinterpret_cast<PyTypeObject*>(contextType.ptr()), 0));
1062 if (!contextOwner)
! 1063 throw py::error_already_set();
1064 auto* context = reinterpret_cast<Context*>(contextOwner.ptr());
1065 context->stateType = stateType.release().ptr();
1066 context->stateKey = PyUnicode_InternFromString("_native_fetch_state");
1067 if (!context->stateKey)
! 1068 throw py::error_already_set();
1069 m.attr("_native_fetch_schema") = 2;
1070 #ifdef MSSQL_NATIVE_FETCH_TEST_FAULTS
1071 m.attr("_native_fetch_test_fault_schema") = 1;
1072 #endifLines 1074-1085 1074 py::tuple entries(4);
1075 for (Py_ssize_t i = 0; i < 4; ++i) {
1076 py::object function = steal(PyCFunction_NewEx(&methods[i], contextOwner.ptr(), nullptr));
1077 if (!function)
! 1078 throw py::error_already_set();
1079 py::object descriptor = steal(PyInstanceMethod_New(function.ptr()));
1080 if (!descriptor)
! 1081 throw py::error_already_set();
1082 PyTuple_SET_ITEM(entries.ptr(), i, descriptor.release().ptr());
1083 }
1084 m.attr("_native_fetch_methods") = std::move(entries);
1085 static PyMethodDef controls[] = {Lines 1096-1104 1096 };
1097 for (auto* method = controls; method->ml_name; ++method) {
1098 auto function = steal(PyCFunction_NewEx(method, contextOwner.ptr(), nullptr));
1099 if (!function)
! 1100 throw py::error_already_set();
1101 m.attr(method->ml_name) = std::move(function);
1102 }
1103
1104 auto observer = std::make_unique<Observer>();Lines 1106-1114 1106 PyErr_Clear();
1107 if (PyErr_WarnEx(PyExc_RuntimeWarning,
1108 "Native fetch prototype disabled: CPython watcher slots unavailable",
1109 1) < 0)
! 1110 throw py::error_already_set();
1111 m.attr("_fetch_observer") = py::none();
1112 };
1113 observer->dict = PyDict_AddWatcher(dict_changed);
1114 if (observer->dict < 0) {Lines 1116-1134 1116 return;
1117 }
1118 observer->type = PyType_AddWatcher(type_changed);
1119 if (observer->type < 0) {
! 1120 PyDict_ClearWatcher(observer->dict);
! 1121 unavailable();
! 1122 return;
! 1123 }
1124 observer->function = PyFunction_AddWatcher(function_changed);
1125 if (observer->function < 0) {
! 1126 PyType_ClearWatcher(observer->type);
! 1127 PyDict_ClearWatcher(observer->dict);
! 1128 unavailable();
! 1129 return;
! 1130 }
1131 // One registration per native-module lifetime. The callbacks retain no
1132 // watched objects; CPython removes registrations at interpreter teardown.
1133 m.attr("_fetch_observer") = py::capsule(
1134 observer.release(), "mssql_python.native_fetch.Observer", [](void* pointer) {Lines 1139-1158 1139 const py::tuple& dicts, const py::tuple& types) {
1140 auto* observer = static_cast<Observer*>(
1141 PyCapsule_GetPointer(capsule.ptr(), "mssql_python.native_fetch.Observer"));
1142 if (!observer)
! 1143 throw py::error_already_set();
1144 for (auto dict : dicts) {
1145 if (PyDict_Watch(observer->dict, dict.ptr()) < 0)
! 1146 throw py::error_already_set();
1147 }
1148 for (auto type : types) {
1149 if (!PyType_Check(type.ptr()))
! 1150 throw py::type_error("Expected a type dependency");
1151 if (!(PyType_GetFlags(reinterpret_cast<PyTypeObject*>(type.ptr())) &
1152 Py_TPFLAGS_IMMUTABLETYPE) &&
1153 PyType_Watch(observer->type, type.ptr()) < 0)
! 1154 throw py::error_already_set();
1155 }
1156 });
1157 }mssql_python/pybind/row_factory.hppLines 16-24 16 const py::tuple& attribute_names) {
17 auto* row_type = reinterpret_cast<PyTypeObject*>(row_class.ptr());
18 py::object row = steal(row_type->tp_alloc(row_type, 0));
19 if (!row)
! 20 throw py::error_already_set();
21 PyObject* fields[] = {values.ptr(), column_map.ptr(), cursor.ptr(),
22 column_map_lower.ptr(), column_names.ptr()};
23 for (Py_ssize_t i = 0; i < 5; ++i) {
24 if (SetAttribute(row.ptr(), PyTuple_GET_ITEM(attribute_names.ptr(), i),Lines 22-30 22 column_map_lower.ptr(), column_names.ptr()};
23 for (Py_ssize_t i = 0; i < 5; ++i) {
24 if (SetAttribute(row.ptr(), PyTuple_GET_ITEM(attribute_names.ptr(), i),
25 fields[i]) < 0)
! 26 throw py::error_already_set();
27 }
28 return row;
29 }📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 62.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 80.2%
mssql_python.pybind.connection.connection_pool.cpp: 81.3%
mssql_python.pybind.row_mapping_cache.hpp: 81.5%
mssql_python.pybind.connection.connection.cpp: 82.5%
mssql_python.logging.py: 86.2%
mssql_python.pybind.row_mapping.hpp: 86.8%🔗 Quick Links
|
Jahnvi Thakkar (jahnvi480)
marked this pull request as draft
October 28, 2026 05:02
Do not force eligible fetches through repeated Python-policy preparation when the profiler is enabled. Keep native fetch attribution on the actual execution path, retain all safety and debug-logging guards, and cover both profiler states in the existing operation-count regression. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Jahnvi Thakkar (jahnvi480)
marked this pull request as ready for review
October 28, 2026 09:14
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Native watcher, GC ownership, and ODBC fetch changes require rebuilt-extension and live SQL Server validation across supported CPython/platform builds that is unavailable here, while current-build performance qualification remains incomplete.
2 open issues
Review effort: Balanced
Preserve shared column-name metadata, bind malformed fetchmany arguments before state access, and handle decimal NULLs without changing the decimal context. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Document that custom column-name subclasses retain the Python fallback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Native watcher, GC ownership, and ODBC fetch changes require rebuilt cross-version, live-SQL Server, lifetime, and fault-injection validation unavailable in this source-only review, while release-performance qualification remains outstanding.
No open issues 🎉
3 resolved since last review
Review effort: Balanced
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
N/A; the work item above is the sole reference.
Summary
Reuse cursor-scoped native state for eligible one-row fetching while retaining the existing Python fallback behavior.
fetchone(), eligiblefetchmany(1),fetchval(), and iteration on supported GIL-enabled CPython versions. Unsupported interpreters retain the Python methods.SQLGetDatametadata, and reuse successful column unbinding.This is the complete 10-file Source05 delta against
9424705251baf8ded41a8a4968216b1fd9b00929, without later hybrid/converter redesigns or benchmark artifacts. The recovered sources match the measured Source05 archive; no current-base semantic adaptation was needed. There is no public API change.Performance scope
Historical matched Linux diagnostics of these Source05 sources used approximately 42.2% less elapsed time for repeated
fetchone()and 74.1% less for repeatedfetchmany(1)than pinned main. Converter fetching was mixed and 1.1% slower at the paired median; pyodbc remained faster in all retained core-operation pairs. These narrow historical diagnostics are not a new benchmark run or all-API qualification. Converter, pyodbc, and full performance-qualification requirements remain unmet; this PR does not claim merge readiness.