PERF: Optimize checked temporal fetch construction by jahnvi480 · Pull Request #795 · microsoft/mssql-python

Jahnvi Thakkar (jahnvi480) · 2026-09-17T14:26:34Z

Work Item / Issue Reference

GitHub Issue: #554

ADO Task: AB#48255

Summary

Reduce per-value Python-call overhead when converting fetched SQL DATE, TIME, and TIMESTAMP values into Python objects. This draft is for cross-platform build/test validation, not a claim of universal performance improvement or merge readiness.

Add purpose-named fetch_temporal.hpp helpers using checked PyDate_FromDate, PyTime_FromTime, and PyDateTime_FromDateAndTime only for the exact standard cached constructor identities.
Preserve substituted constructors through the original callable path, including 3/4/7 positional arguments, returned objects, and propagated exceptions.
Initialize the translation unit's datetime C API safely; keep helpers translation-unit-local and preserve existing GIL scopes and new-reference ownership.
Wire only six row-wise/batch temporal construction sites. Preserve SQL/NULL checks, buffers, fraction/1000 truncation, naive timezone/fold behavior, and result positioning. DATETIMEOFFSET, UUID, Decimal, text, Arrow, Row construction, and settings/converter caches are unchanged.
Add isolated fresh-process temporal regression coverage and a changelog entry.

Local validation

Windows x64, CPython 3.13.15, pybind11 3.0.1, MSVC Release with profiling compiled out; SQL Server 15.0.4382.1 and ODBC 18.6.2.1.

Freshly built pristine baseline and candidate both pass six default/custom/throwing constructor modes, with 24 configurations per mode. Checks cover DATE boundaries 0001/9999/leap day, TIME(7)/DATETIME2(7) truncation, datetime/smalldatetime boundaries, NULLs, exact types, timezone/fold, and unchanged scalar controls.
Bounded and MAX-forced row-wise results are exercised through fetchone, fetchmany, fetchall, and iteration. Throwing modes preserve the exact exception instance and verify cursor recovery 16 times per mode; process teardown succeeds.
40 selected existing temporal/fetch regression tests pass on each build. Black checks pass for 91 Python files; git diff whitespace checks pass.
No full-suite, other-platform, sanitizer, or allocation-failure-injection success is claimed. The existing Azure DevOps pipeline defines Windows/macOS/Linux validation and Linux ARM64 jobs; actual run outcomes still need review.

Performance observations and limitations

Matched uninstrumented baseline/candidate measurements used 12 paired rounds with nine samples per case, retaining all ordered samples. One predeclared confirmation was run after noisy calibration. These figures describe this actual temporal-only source, not an integrated optimization branch.

Confirmation workload	Baseline -> candidate fetch ms	Reduction	Paired 95% reduction interval
DATE, fetchone	44.977 -> 38.488	14.43%	6.20% to 24.87%
DATE, fetchmany(1000)	12.935 -> 9.732	24.76%	17.14% to 31.12%
DATE, fetchall	11.675 -> 8.961	23.25%	-3.30% to 29.43%
Mixed temporal, fetchall	17.423 -> 13.622	21.82%	9.04% to 26.59%

The 15-case confirmation sum of fetch medians decreased 531.062 -> 506.612 ms (4.60%); execute-plus-drain decreased 543.121 -> 518.142 ms (4.60%). These are sums of fixed workload medians, not an application speedup. Separate pure TIME/TIMESTAMP cases also produced positive observations, but are not added to these percentages.

Identical-build A/A calibration was very noisy (paired score interval 0.828x to 1.951x). Negative unchanged confirmation controls are retained: narrow fetchone -1.19%, many(1) -1.14%, fetchall -2.79%, converter many -4.63%, converter all -2.39%; their intervals span zero. No dependable blanket no-regression conclusion is established.

Retained validation limitations

The original shared scalar probe passed its first 12 controls, then failed an unrelated baseline leading-BOM assertion. This PR does not include the independent BOM fix; the scoped temporal tests retain scalar controls without that unrelated requirement.
An initial existing-test selection omitted the table-setup tests and produced eight missing-table failures; the matched selection including the required setup passes 40 tests on both builds.
Both native builds retained the pre-existing LNK4044 warning. No unrelated build-system changes are included.

Use direct CPython date/time/datetime construction for exact cached standard types, preserving substituted constructors and exception behavior. Cover row-wise and batch fetch contracts in isolated subprocesses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

Native extension changes span multiple fetch paths, with limited platform and validation coverage.

Pull request overview

Optimizes native SQL temporal fetch construction while preserving custom constructors and conversion behavior.

Changes:

Adds checked datetime construction helpers.
Integrates them into six temporal fetch paths.
Adds regression tests and changelog documentation.

File summaries

File	Reviewed changes
`tests/test_038_fetch_temporal.py`	Temporal parity, constructor, exception, and fetch API coverage
`mssql_python/pybind/fetch_temporal.hpp`	Translation-unit-local checked construction helpers
`mssql_python/pybind/ddbc_bindings.cpp`	Integration into row-wise and batch temporal fetch paths
`CHANGELOG.md`	Documents the optimization and preserved behavior

Review details

Files reviewed: 4/4 changed files
Comments generated: 0
Review effort level: Lite

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

github-actions · 2026-09-17T14:34:23Z

📊 Code Coverage Report

🔥 Diff Coverage 90%	🎯 Overall Coverage 83%	📈 Total Lines Covered: `8475` out of `10120` 📁 Project: `mssql-python`

Diff Coverage

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

mssql_python/pybind/ddbc_bindings.cpp (84.6%): Missing lines 4447-4448
mssql_python/pybind/fetch_temporal.hpp (92.1%): Missing lines 17-19

Summary

Total: 51 lines
Missing: 5 lines
Coverage: 90%

mssql_python/pybind/ddbc_bindings.cpp

Lines 4443-4452

  4443                 }
  4444                 case SQL_SS_TIME2: {
  4445                     const SQL_SS_TIME2_STRUCT& t2 = buffers.timeBuffers[col - 1][i];
  4446                     py::object timeObj =
! 4447                         FetchTemporal::time(t2.hour, t2.minute, t2.second, t2.fraction / 1000);
! 4448                     PyList_SET_ITEM(row, col - 1, timeObj.release().ptr());
  4449                     break;
  4450                 }
  4451                 case SQL_SS_TIMESTAMPOFFSET: {
  4452                     SQLULEN rowIdx = i;

mssql_python/pybind/fetch_temporal.hpp

Lines 13-23

  13 
  14 // datetime.h keeps PyDateTimeAPI per translation unit, so these helpers must too.
  15 static inline void ensure_datetime_api() {
  16     if (PyDateTimeAPI == nullptr) {
! 17         PyDateTime_IMPORT;
! 18         if (PyDateTimeAPI == nullptr) throw py::error_already_set();
! 19     }
  20 }
  21 
  22 static inline py::object date(int year, int month, int day) {
  23     ensure_datetime_api();

📋 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: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.row.py: 77.6%
mssql_python.pybind.ddbc_bindings.cpp: 77.9%
mssql_python.pybind.connection.connection_pool.cpp: 81.8%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.cursor.py: 92.1%

🔗 Quick Links

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

Copilot AI lite review requested due to automatic review settings September 17, 2026 14:26

Copilot started reviewing on behalf of Jahnvi Thakkar (jahnvi480) September 17, 2026 14:27 View session

Jahnvi Thakkar (jahnvi480) changed the title ~~REFACTOR: Optimize checked temporal fetch construction~~ PERF: Optimize checked temporal fetch construction Sep 17, 2026

Copilot AI reviewed Sep 17, 2026

View reviewed changes

github-actions Bot added the pr-size: medium Moderate update size label Sep 17, 2026

Navigation Menu

PERF: Optimize checked temporal fetch construction - #795

PERF: Optimize checked temporal fetch construction#795
Jahnvi Thakkar (jahnvi480) wants to merge 1 commit into
mainfrom
jahnvi/perf-fetch-temporal-construction

Jahnvi Thakkar (jahnvi480) commented Sep 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Sep 17, 2026

🔥 Diff Coverage

90%

🎯 Overall Coverage

83%

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Jahnvi Thakkar (jahnvi480) commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Work Item / Issue Reference

Summary

Local validation

Performance observations and limitations

Retained validation limitations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

github-actions Bot commented Sep 17, 2026

📊 Code Coverage Report

🔥 Diff Coverage

90%

🎯 Overall Coverage

83%

Diff Coverage

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

Summary

mssql_python/pybind/ddbc_bindings.cpp

mssql_python/pybind/fetch_temporal.hpp

📋 Files Needing Attention

🔗 Quick Links

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Jahnvi Thakkar (jahnvi480) commented Sep 17, 2026 •

edited

Loading