You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
PERF: Optimize fetchone, fetchmany(1) and fetchval paths - #829
Not applicable; the work item above is the single reference.
Summary
Consolidate fetchone(), fetchmany(1) and inherited fetchval()/iterator optimization work in this existing PR.
Retain generation-scoped full column-count caching, separate from prefix SQLGetData metadata. Direct column-count calls remain uncached; all nine original count-cache regressions remain.
Share native single-row fetching and reuse successful unbinds only within a valid generation. Invalidate before binding, including Arrow/partial binds, and during cleanup. The marker does not certify row-array attributes.
Route only all-numeric fetchmany(1) results through SQLFetchScroll and SQLGetData, preserving eager count/name validation and row-array configuration/cleanup. Mixed INT/NVARCHAR and other types retain their existing native paths.
Use direct one-row wrapping only for exact built-in integer size 1, one returned row, canonical Row/factory, and no converter/UUID work. Preserve larger-request tails, substituted factories, integer subclasses and actual fetchone() overrides.
Keep full-row construction and all-column converter callbacks for fetchval(). Replace Python single-row phase context managers with equivalent paired start/stop instrumentation.
Add numeric parity, EOF, override/factory, diagnostics, generation-change, mixed-API and fault-recovery regressions, plus attribution documentation.
Unix / SQL Server 2022: Python 3.12.3, x86_64, SQL 16.0.4295.3; 5 paired comparisons and 1 warmup.
Unix / SQL Server 2025: Python 3.12.3, x86_64, SQL 17.0.5005.3; 5 paired comparisons and 1 warmup.
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.
This headline uses the original 22-task profiling-enabled diagnostics; separate OFF/OFF latency and ON/OFF route measurements, when available, are retained in the raw artifacts and are not headline inputs.
Raw samples and logs are attached to the ADO run as profiler-* artifacts.
Use a separate connection for the cross-handle assertion without requiring MARS. Preserve all fetch and native call-count assertions.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2975column_names=self._cached_result_columns,
2976 )
2977finally:
2978ifstarted:
! 2979perf_stop("py::fetchone::row_wrap", started)
29802981deffetchmany(self, size: Optional[int] =None) ->List[Row]:
2982 """
2983 Fetch the next set of rows of a query result.
3843// null termination. This preserves embedded NULs and avoids3844// any risk of reading past the valid range if the driver3845// omits the terminator.3846 row.append(FetchText::from_utf16_native(
! 3847reinterpret_cast<constchar*>(dataBuffer),
3848static_cast<Py_ssize_t>(numCharsInData * sizeof(SQLWCHAR))));
3849LOG("SQLGetData: Appended NVARCHAR string "3850"length=%lu for column %d",
3851 (unsignedlong)numCharsInData, i);
1112// Only the post-fetch cached-map loop: no ODBC access, Row allocation, or UUID work.13inline py::list apply_output_converters(const py::object& values, const py::object& converters) {
14if (!PyList_CheckExact(values.ptr()) || !PyList_CheckExact(converters.ptr())) {
! 15throwpy::type_error("converter values and map must be exact lists");
! 16 }
17 py::list result =
18 steal<py::list>(PyList_GetSlice(values.ptr(), 0, PyList_GET_SIZE(values.ptr())));
19if (!result)
! 20throwpy::error_already_set();
2122// Keep the Python iterators: their retained tuples affect finalizer timing23// when a callback replaces itself or mutates the source lists.24 py::object pairs = steal(PyObject_CallFunctionObjArgs(reinterpret_cast<PyObject*>(&PyZip_Type),
Lines 23-35
23// when a callback replaces itself or mutates the source lists.24 py::object pairs = steal(PyObject_CallFunctionObjArgs(reinterpret_cast<PyObject*>(&PyZip_Type),
25 values.ptr(), converters.ptr(), nullptr));
26if (!pairs)
! 27throwpy::error_already_set();
28 py::object items =
29steal(PyObject_CallOneArg(reinterpret_cast<PyObject*>(&PyEnum_Type), pairs.ptr()));
30if (!items)
! 31throwpy::error_already_set();
32 pairs = py::object();
3334// Retain the current inputs and last encoded value like the Python locals.35 py::object value, converter, value_bytes;
Jahnvi Thakkar (jahnvi480)
changed the title
PERF: Cache full column counts for fetchone
PERF: Optimize fetchone, fetchmany(1) and fetchval paths
Oct 5, 2026
Add regression tests for closed source recognition
eng/profiler_benchmarks/controller.py:294
The new closed source recognizer has no direct regression coverage: the aggregate test monkeypatches python_source_identity, and no test invokes this function. A stale digest or an AST false rejection would make every latency/route worker fail before measurement. Add no-DB tests covering the current cursor source, a supported legacy source without the declaration, and representative duplicate/rebinding rejection cases.
Avoid descriptor invocation when resolving _fast_create
mssql_python/cursor.py:3066
This check resolves _fast_create through Python's descriptor protocol after the native fetch has already advanced the cursor. A replacement descriptor can therefore run side effects or raise here, making fetchmany(1) consume a row and fail even though the previous batch wrapper never consulted that factory. Inspect the raw class dictionary, as _native_row_eligible does, so substituted descriptors reliably stay on the batch path.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The profiler documentation attributes native fusion to a nonexistent test and an inactive default route.
0 open findings
Previously missed (1)
In code that hasn't changed since last review
Update attribution docs for inactive binding and actual route checks
profiler/README.md:248
This attribution section describes a phase-controlled fused route and test_single_row_fusion_native_counters_in_subprocess, but neither exists in the current source: ordinary fetches call the split helpers with native=False, and the test name appears only here. The documented counters therefore cannot be produced by the current default path; update this block to describe the inactive binding and the route-mode checks that actually run.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The extensive native ownership, ODBC state, and CI reporting changes require final human review and exact-head cross-platform validation.
0 open findings
🧠 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
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.
github-actions
Bot
added
pr-size: large
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Work Item / Issue Reference
Not applicable; the work item above is the single reference.
Summary
fetchone(),fetchmany(1)and inheritedfetchval()/iterator optimization work in this existing PR.SQLGetDatametadata. Direct column-count calls remain uncached; all nine original count-cache regressions remain.fetchmany(1)results throughSQLFetchScrollandSQLGetData, preserving eager count/name validation and row-array configuration/cleanup. Mixed INT/NVARCHAR and other types retain their existing native paths.fetchone()overrides.fetchval(). Replace Python single-row phase context managers with equivalent paired start/stop instrumentation.