PERF: Optimize Row construction and repeated cursor bookkeeping

[Jahnvi Thakkar (jahnvi480)]

Work Item / Issue Reference

AB#44921

Summary

Reduce Python-side overhead in fetchone(), fetchmany(), and fetchall() while preserving decoding, converter, diagnostic, and Row-access behavior. The branch incorporates main through e279a4f6 and includes the review fixes in 56c3d3b5.

Fetch optimizations

• Construct eligible fetchmany()/fetchall() rows in native construct_rows, bypassing the Python per-row initialization loop. Use Row._fast_create() for the corresponding fetchone() path.
• Add Row.__slots__ and retain zero-copy storage of fetched values when no conversion is required. Initialize the shared lowercase column map on both Python and native construction paths.
• Cache CHAR/WCHAR encoding strings and the CHAR C type. Refresh them only when connection decoding settings change, including for existing cursors.
• Reuse the SQL-to-C type mapping and use str.isascii() for Unicode detection.
• Keep the no-conversion fast path available when converters are registered but none applies to the current result set. An explicit empty mapping avoids unnecessary row copies or per-row fallback lookups.

Correctness and compatibility

• Track converter-generation changes so adding, replacing, removing, or clearing converters after execute() updates subsequent fetches without rebuilding mappings on unchanged cache hits.
• Preserve current-main dispatch precedence: raw ODBC SQL type, Python type, then the string/bytes-only WVARCHAR fallback. Unrelated integer and UUID columns are not passed to the string fallback.
• Preserve direct Row construction's connection-converter fallback when no precomputed mapping is supplied, including UUID stringification.
• Retain main's lowercase/string-key column access, profiling scopes, and CHAR decoding C-type forwarding. Native WCHAR decoding semantics are unchanged.
• Validate row_class with PyType_Check before casting in the Python-callable native helper. Invalid arguments raise TypeError rather than crashing the interpreter.
• Restore unconditional diagnostic retrieval after non-error fetch returns. The native wrappers can overwrite intermediate statuses, so checking only the final SQL_SUCCESS_WITH_INFO is unsafe. The earlier diagnostic-skip optimization has been removed; this restores prior behavior rather than claiming to repair pre-existing native diagnostic-record overwrites.
• Check errors in fetchone() and fetchmany() before updating row positions or constructing rows, consistent with fetchall().

Performance: current main versus this PR

Workload (rows x columns)	API	Main median	PR median	Time reduction
Narrow: 10,000 x 3	fetchmany(1000)	9.36 ms	6.54 ms	30.18%
Narrow: 10,000 x 3	fetchall()	8.67 ms	6.99 ms	19.30%
Wide: 10,000 x 24	fetchmany(1000)	27.57 ms	24.72 ms	10.34%
Wide: 10,000 x 24	fetchall()	30.59 ms	26.07 ms	14.79%

Positive reduction means less time, calculated from unrounded medians. Narrow/wide batch cases improved in 10/10 and 9/10 pairs respectively. fetchmany(1) medians were only 1.15-1.80% lower, with narrow results inconclusive. Narrow fetchone() was 5.02% lower; wide fetchone().

These are workload-specific results, not a universal or cross-platform speedup. Full results for all 11 fetch cases, control, dispersion, paired intervals, build identities, and raw repetitions are retained locally; older pre-merge numbers are not mixed in.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 94%

🎯 Overall Coverage 84%

📈 Total Lines Covered: 8780 out of 10426 📁 Project: mssql-python

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql_python/connection.py (100%)
• mssql_python/cursor.py (100%)
• mssql_python/pybind/ddbc_bindings.cpp (82.9%): Missing lines 3635-3637,3651-3653,3656
• mssql_python/pybind/row_factory.hpp (97.1%): Missing lines 38
• mssql_python/row.py (100%)

Summary

• Total: 144 lines
• Missing: 8 lines
• Coverage: 94%

mssql_python/pybind/ddbc_bindings.cpp

Lines 3631-3641

  3631             case SQL_SMALLINT: {
  3632                 SQLSMALLINT smallIntValue;
  3633                 SQLLEN indicator = 0;
  3634                 ret = SQLGetData_ptr(hStmt, i, SQL_C_SHORT, &smallIntValue, 0, &indicator);
! 3635                 if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
! 3636                     row.append(py::none());
! 3637                     break;
  3638                 }
  3639                 if (SQL_SUCCEEDED(ret)) {
  3640                     row.append(static_cast<int>(smallIntValue));
  3641                 } else {

---

Lines 3647-3660

  3647                 break;
  3648             }
  3649             case SQL_REAL: {
  3650                 SQLREAL realValue;
! 3651                 SQLLEN indicator = 0;
! 3652                 ret = SQLGetData_ptr(hStmt, i, SQL_C_FLOAT, &realValue, 0, &indicator);
! 3653                 if (SQL_SUCCEEDED(ret) && indicator == SQL_NULL_DATA) {
  3654                     row.append(py::none());
  3655                     break;
! 3656                 }
  3657                 if (SQL_SUCCEEDED(ret)) {
  3658                     row.append(realValue);
  3659                 } else {
  3660                     LOG("SQLGetData: Error retrieving SQL_REAL for column %d - "

---

mssql_python/pybind/row_factory.hpp

Lines 34-42

  34 
  35     for (Py_ssize_t i = 0; i < n; ++i) {
  36         py::object row = steal(row_type->tp_alloc(row_type, 0));
  37         if (!row)
! 38             throw py::error_already_set();
  39 
  40         PyObject* row_data = PyList_GET_ITEM(rows_data.ptr(), i);
  41 
  42         if (PyObject_GenericSetAttr(row.ptr(), attr_values.ptr(), row_data) < 0 ||

---

---

📋 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: 64.1%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 78.4%
mssql_python.pybind.connection.connection_pool.cpp: 82.3%
mssql_python.pybind.connection.connection.cpp: 82.5%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.pybind.py_type_cache.hpp: 91.6%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

[Copilot]

🟡 Changes recommended

Fetch diagnostics, public Row compatibility, and C++ input validation require fixes.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR optimizes fetch performance through cached settings, faster Row construction, and C++ bulk row creation.

Changes:

• Adds cache invalidation and optimized fetch paths.
• Introduces slotted rows and fast construction.
• Adds C++ bulk row construction and regression tests.

File summaries

File	Summary
tests/test_fetch_settings_cache.py	Tests cache behavior and fast paths.
mssql_python/row.py	Adds slots and optimized row creation.
mssql_python/pybind/ddbc_bindings.cpp	Implements C++ row construction.
mssql_python/cursor.py	Uses cached settings and optimized fetch paths.
mssql_python/connection.py	Tracks configuration generations.

Review details

Suppressed comments (5)

mssql_python/cursor.py:2571

• DDBCSQLFetchMany returns a negative SQLRETURN for binding/fetch failures, but this method does not otherwise call check_error; this new gate skips the diagnostics and the code then proceeds to construct rows from whatever partial data is present. Check the return code before row-count and row construction, while keeping the SQL_SUCCESS_WITH_INFO diagnostic path.

            if ret == ddbc_sql_const.SQL_SUCCESS_WITH_INFO.value and self.hstmt:
                self.messages.extend(ddbc_bindings.DDBCSQLGetAllDiagRecords(self.hstmt))

mssql_python/cursor.py:2500

• This guard can hide diagnostics returned by SQLFetch: the DDBCSQLFetchOne bridge overwrites the SQLFetch return code with SQLGetData's final status, so SQL_SUCCESS_WITH_INFO from the fetch (or an earlier column) is no longer observable here. Before this change the unconditional call preserved those records. Please propagate an info flag through the bridge and retrieve diagnostics when either operation reports it.

            if ret == ddbc_sql_const.SQL_SUCCESS_WITH_INFO.value and self.hstmt:

mssql_python/cursor.py:2570

• For the LOB path, FetchMany_wrap returns SQL_SUCCESS even when its per-row SQLFetch or SQLGetData calls returned SQL_SUCCESS_WITH_INFO, so this guard skips their diagnostic records. Please make the bridge preserve an aggregate info status (or otherwise signal it) before gating DDBCSQLGetAllDiagRecords.

            if ret == ddbc_sql_const.SQL_SUCCESS_WITH_INFO.value and self.hstmt:

mssql_python/cursor.py:2636

• FetchAll_wrap loops until SQL_NO_DATA and returns that final status, even if an earlier FetchBatchData call returned SQL_SUCCESS_WITH_INFO; its LOB path also returns SQL_SUCCESS after discarding intermediate statuses. Consequently this condition is false for most fetchall warnings and regresses cursor.messages. Please preserve an aggregate info status in FetchAll_wrap and use it here.

            if ret == ddbc_sql_const.SQL_SUCCESS_WITH_INFO.value and self.hstmt:

mssql_python/row.py:32

• Row is exported as mssql_python.Row, so adding __slots__ removes observable existing behavior: callers can no longer assign non-column attributes, inspect row.__dict__, or weak-reference rows. Preserve the prior instance API (which reduces the memory win) or explicitly treat this as a breaking public API change rather than silently applying it to every fetched row.

    __slots__ = ("_values", "_column_map", "_cursor")

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Moderate issues remain in diagnostic preservation, converter fast-path handling, and native argument validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

mssql_python/cursor.py:2889

• The LOB branch of DDBCSQLFetchMany returns SQL_SUCCESS after its loop even when an individual SQLFetch returned SQL_SUCCESS_WITH_INFO. Therefore this new condition never drains diagnostics for warnings encountered while fetching LOB rows. The native bridge needs to propagate or drain an intermediate info status before this optimization can safely replace the unconditional drain.

                if ret == ddbc_sql_const.SQL_SUCCESS_WITH_INFO.value and self.hstmt:
                    self.messages.extend(ddbc_bindings.DDBCSQLGetAllDiagRecords(self.hstmt))

mssql_python/cursor.py:2961

• DDBCSQLFetchAll does not expose the status of its internal fetches: the batch loop ends with SQL_NO_DATA, while its LOB path explicitly returns SQL_SUCCESS. Thus an SQL_SUCCESS_WITH_INFO from an earlier batch/row is not visible here and its diagnostic records are skipped, unlike the old unconditional drain. Aggregate/propagate or drain diagnostics inside the native bridge before gating this call on the final return code.

                if ret == ddbc_sql_const.SQL_SUCCESS_WITH_INFO.value and self.hstmt:
                    self.messages.extend(ddbc_bindings.DDBCSQLGetAllDiagRecords(self.hstmt))

mssql_python/cursor.py:1416

• When any output converter is registered but none matches this result set, this returns a truthy list containing only None values. The new fast-path checks therefore fail in all three fetch methods, and Row._apply_output_converters_optimized copies every row even though no conversion is needed. Return None (or otherwise record whether the map contains a converter) for an all-None map so unrelated converters do not impose per-row overhead.

        self._cached_converters_generation = generation
        return converter_map

mssql_python/pybind/ddbc_bindings.cpp:6054

• This newly exposed native function casts any Python object to PyTypeObject* and dereferences it; for example, construct_rows([[1]], None, {}, None) can dereference a null pointer and crash the interpreter instead of raising a Python exception. Validate PyType_Check(row_class.ptr()) before the cast.

    PyTypeObject* row_type = reinterpret_cast<PyTypeObject*>(row_class.ptr());

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

A critical SQL_SS_VARIANT converter-dispatch issue and a moderate Row API compatibility issue remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

mssql_python/row.py:31

• Row is exported as a public class (mssql_python/__init__.py:68,357) and previously had normal instance semantics. Adding slots here removes row.__dict__/arbitrary attribute assignment and weak-reference support for every fetched row, so consumers that attach metadata or use weakref.ref(row) now fail with AttributeError/TypeError. Please preserve those capabilities (for example, by including __dict__ and __weakref__) or explicitly document and version this as a breaking API change rather than an optimization-only change.

    __slots__ = ("_values", "_column_map", "_cursor", "_column_map_lower")

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Native bindings, caching, conversion, and diagnostics are affected, and the full suite and cross-platform matrix were not run.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[github-actions]

PR Performance Report

This PR has 4 consistent improvement signals across 2 database tasks and 2 environments.

Environment	Affected task	Before	After	Change
Unix / SQL Server 2022	Insertion with explicit input sizes	2219.571 ms	454.543 ms	-78.9%
Unix / SQL Server 2022	1.2-million-row fetching	5227.590 ms	3467.842 ms	-33.7%
Unix / SQL Server 2025	Insertion with explicit input sizes	2365.944 ms	492.311 ms	-79.2%
Unix / SQL Server 2025	1.2-million-row fetching	5053.413 ms	3464.910 ms	-31.9%

Coverage: 2 of 2 environments completed. Advisory result; does not block merging.

Environment	Status
Unix / SQL Server 2022	Completed
Unix / SQL Server 2025	Completed

Affected phases and call counts

Phase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed.

Unix / SQL Server 2022

Insertion with explicit input sizes: py::execute::cpp_call -30.720 ms; ddbc::SQLExecute_wrap -30.082 ms; ddbc::BindParameters -10.304 ms.
1.2-million-row fetching: py::fetchall::row_wrap -1772.258 ms; ddbc::FetchBatchData::construct_rows -38.089 ms; ddbc::FetchBatchData -5.790 ms.

Unix / SQL Server 2025

Insertion with explicit input sizes: py::execute::cpp_call -17.528 ms; ddbc::SQLExecute_wrap -16.864 ms; ddbc::BindParameters -7.866 ms.
1.2-million-row fetching: py::fetchall::row_wrap -1563.375 ms; ddbc::FetchBatchData::construct_rows -38.919 ms; py::fetchall::cpp_call -31.630 ms.

All database tasks and timings

Unix / SQL Server 2022

Database task	Before	After	Paired change	Result
Connection opening	10.950 ms	10.962 ms	+0.1%	no signal
SELECT queries	1.143 ms	1.305 ms	+16.9%	no signal
Row insertion	31.593 ms	32.007 ms	+2.6%	no signal
Executemany inserts	137.770 ms	137.155 ms	-0.4%	no signal
Fetch-all queries	169.674 ms	140.275 ms	-16.3%	no signal
Row-by-row fetching	52.428 ms	49.988 ms	-4.2%	no signal
Batched row fetching	154.448 ms	138.428 ms	-11.4%	no signal
Transaction commit and rollback	99.866 ms	100.484 ms	-0.6%	no signal
Arrow row fetching	90.944 ms	92.474 ms	+0.1%	no signal
100,000-row insertion	405.113 ms	413.220 ms	-0.3%	no signal
Row fetching in batches of 100	200.748 ms	177.094 ms	-13.3%	no signal
Row fetching in batches of 10,000	178.199 ms	166.882 ms	-6.9%	no signal
Repeated positional queries	38.976 ms	38.422 ms	-0.7%	no signal
Repeated named-parameter queries	41.153 ms	41.123 ms	-0.0%	no signal
Legacy 100,000-row insertion	314.417 ms	327.583 ms	+1.4%	no signal
Insertion with explicit input sizes	2219.571 ms	454.543 ms	-78.9%	consistent improvement
Joined aggregation queries	187.493 ms	184.906 ms	-1.6%	no signal
Large joined-result fetching	216.007 ms	202.234 ms	-7.8%	no signal
1.2-million-row fetching	5227.590 ms	3467.842 ms	-33.7%	consistent improvement
Common table expression queries	5.760 ms	5.721 ms	-2.2%	no signal

Unix / SQL Server 2025

Database task	Before	After	Paired change	Result
Connection opening	97.983 ms	97.239 ms	-0.2%	no signal
SELECT queries	1.193 ms	1.124 ms	-5.0%	no signal
Row insertion	34.724 ms	34.591 ms	-0.4%	no signal
Executemany inserts	153.403 ms	151.354 ms	-0.9%	no signal
Fetch-all queries	175.954 ms	143.873 ms	-18.1%	no signal
Row-by-row fetching	62.727 ms	56.390 ms	-8.9%	no signal
Batched row fetching	170.401 ms	146.021 ms	-13.1%	no signal
Transaction commit and rollback	115.855 ms	116.488 ms	+0.0%	no signal
Arrow row fetching	93.924 ms	95.359 ms	+0.7%	no signal
100,000-row insertion	463.815 ms	452.990 ms	-4.5%	no signal
Row fetching in batches of 100	226.449 ms	197.217 ms	-12.2%	no signal
Row fetching in batches of 10,000	188.183 ms	166.346 ms	-10.5%	no signal
Repeated positional queries	42.194 ms	41.938 ms	-0.6%	no signal
Repeated named-parameter queries	44.670 ms	44.430 ms	-0.5%	no signal
Legacy 100,000-row insertion	380.245 ms	355.640 ms	-6.4%	no signal
Insertion with explicit input sizes	2365.944 ms	492.311 ms	-79.2%	consistent improvement
Joined aggregation queries	161.580 ms	163.189 ms	+1.2%	no signal
Large joined-result fetching	214.604 ms	205.337 ms	-2.3%	no signal
1.2-million-row fetching	5053.413 ms	3464.910 ms	-31.9%	consistent improvement
Common table expression queries	5.235 ms	5.228 ms	-0.1%	no signal

Build, commits and measurement details

ADO build 177117

PR head: 6bc28017518ec1a4a8740f2dc54cb990a579ae7d
Base: f3e34e66bca6d6f45d7a2cbea070c40ebe4ba194
Measured merge: 1dd6826c51930f9542da2ad627a7dac3a8cbc8a0

• 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.

Raw samples and logs are attached to the ADO run as profiler-* artifacts.

[Copilot]

🔵 Needs a closer look

Row.__slots__ may break public attribute and weak-reference compatibility; this should be addressed before approval.

Review details

Suppressed comments (1)

mssql_python/row.py:31

• Row is exported as mssql_python.Row (mssql_python/__init__.py:67-68), so adding __slots__ changes the public object contract: instances no longer accept user attributes and are no longer weak-referenceable. Existing callers that attach metadata (row.foo = ...) or use weakref.ref(row) will now fail even though column access is unchanged. Preserve compatibility (for example by retaining __dict__ and __weakref__, with the corresponding memory trade-off) or explicitly treat this as a documented breaking API change.

    __slots__ = ("_values", "_column_map", "_cursor", "_column_map_lower")

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Copilot review overview

🔵 Needs a closer look

Resolve the native reference leak and preserve or explicitly address Row instance-dictionary and weak-reference compatibility.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

Native fetch, caching, and compatibility changes warrant final human review.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

Broad performance and native fetch-path changes warrant final human review.

Review effort: Lite
Findings: None

[Copilot]

Copilot review overview

🔵 Needs a closer look

Three unresolved cache-invalidation findings remain in addition to the native fetch-path changes.

Review effort: Lite
Findings: None

[Sumit Sarabhai (sumitmsft)]

PR #558: Both previously reported findings are addressed. Late string-converter registration preserves non-string variant values and explicit converter precedence. Native Row construction rejects incompatible types before allocation and handles reference ownership and failure cleanup safely.

No remaining actionable findings in the reviewed changes.