Skip to content

FEAT: TVP support for async queries - #814

Open
Subrata (subrata-ms) wants to merge 12 commits into
mainfrom
subrata-ms/AQEIntegration
Open

Subrata (subrata-ms) wants to merge 12 commits into
mainfrom
subrata-ms/AQEIntegration

Conversation

@subrata-ms

@subrata-ms Subrata (subrata-ms) commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#47998,48435

GitHub Issue: #<ISSUE_NUMBER>


Summary

This pull request introduces several improvements and enhancements to the mssql_python.async_query async API, focusing on better native type integration, more robust and consistent cursor behavior, and improved handling of parameter iterables in executemany. It also expands the test suite to cover these changes and edge cases.

Key highlights include:

  • Lazy exposure of native SQL type hints and the table-valued parameter constructor.
  • Improved type and error handling for parameter iterables in executemany.
  • New and clarified properties and behaviors for async cursors, including a closed property.
  • Expanded and refined test coverage for new and existing features.

Native type integration and exports

  • Added lazy exports for native SQL type hints (SQL_MONEY, SQL_SMALLMONEY, SQL_XML, SQL_JSON, SQL_VECTOR) and the internal _TableValuedParameter constructor, with dynamic loading and error reporting if the native dependency is missing. These are now included in __all__ and dir() as appropriate.
  • Added tests to verify that these native exports are correctly exposed, loaded only when accessed, and report missing features properly.

AsyncCursor API and behavior improvements

  • Added a closed property to async cursors to reflect both wrapper and parent connection state, with tests for idempotency and parent connection closure propagation.
  • Clarified and enhanced docstrings for nextset, close, description, and rowcount to document async-specific behaviors and differences from the synchronous API.
  • Added tests to ensure nextset correctly tracks rowcount and description per result, including edge cases.

Executemany and parameter iterable handling

  • Changed executemany signatures to accept any synchronous iterable (not just sequences), improved error handling for non-iterables, and ensured iterables are consumed only once. Streaming/asynchronous iterables are explicitly not supported.
  • Added tests for handling of empty generators, iterator failures, and correct consumption of parameter iterables.

Logging and test adjustments

  • Updated logging to remove reliance on batch_count for executemany and adjusted test expectations accordingly.
  • Minor imports and test harness updates to support new features.

These changes make the async query API more robust, Pythonic, and consistent with both native and DB-API expectations.

Expose native TVP and SQL type-hint helpers lazily. Support iterable executemany batches and preserve result state on iteration failure. Add cursor closed-state reporting, document async result contracts, and cover these behaviors with async integration tests.
Copilot AI lite review requested due to automatic review settings September 24, 2026 10:08
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

PR Performance Report

✅ No regression detected

No consistent slowdowns detected across all 2 environments.

0 IMPROVEMENTS 0 SLOWDOWNS 2/2 ENVIRONMENTS

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

Performance diagnostics

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

No affected phases or call-count changes were recorded.

All database tasks and timings

Unix / SQL Server 2022

Database task Before After Paired change Result
Connection opening 10.417 ms 10.057 ms -2.6% no signal
SELECT queries 1.107 ms 1.082 ms -1.2% no signal
Row insertion 34.903 ms 35.293 ms +2.1% no signal
Executemany inserts 161.558 ms 162.840 ms +3.5% no signal
Fetch-all queries 121.179 ms 121.438 ms +1.1% no signal
Row-by-row fetching 14.754 ms 14.429 ms -2.9% no signal
Batched row fetching 117.767 ms 117.564 ms -0.2% no signal
Transaction commit and rollback 115.650 ms 115.085 ms -1.0% no signal
Arrow row fetching 93.799 ms 94.909 ms +1.1% no signal
100,000-row insertion 436.454 ms 441.087 ms +0.6% no signal
Row fetching in batches of 100 121.423 ms 122.626 ms +0.4% no signal
Row fetching in batches of 10,000 131.567 ms 126.424 ms -1.9% no signal
Repeated positional queries 34.209 ms 33.724 ms -1.3% no signal
Repeated named-parameter queries 36.526 ms 35.859 ms -1.1% no signal
Legacy 100,000-row insertion 355.941 ms 350.725 ms -2.2% no signal
Insertion with explicit input sizes 484.756 ms 491.036 ms +1.8% no signal
Joined aggregation queries 182.665 ms 182.985 ms -0.2% no signal
Large joined-result fetching 181.448 ms 183.419 ms +2.0% no signal
1.2-million-row fetching 3449.398 ms 3484.997 ms +0.7% no signal
Common table expression queries 5.457 ms 5.447 ms -0.3% no signal
256 KiB VARCHAR(MAX) / fetchall() 1.345 ms 1.279 ms -3.6% no signal
10,000 scalar values / fetchval() (debug disabled) 106.903 ms 105.982 ms -0.4% no signal

Unix / SQL Server 2025

Database task Before After Paired change Result
Connection opening 96.779 ms 97.108 ms +0.2% no signal
SELECT queries 1.042 ms 1.072 ms +0.1% no signal
Row insertion 34.110 ms 34.249 ms -0.4% no signal
Executemany inserts 151.788 ms 150.385 ms -1.4% no signal
Fetch-all queries 122.155 ms 120.739 ms +0.3% no signal
Row-by-row fetching 14.552 ms 14.305 ms -1.1% no signal
Batched row fetching 118.070 ms 119.838 ms +0.6% no signal
Transaction commit and rollback 114.324 ms 114.342 ms +0.3% no signal
Arrow row fetching 94.666 ms 94.081 ms -1.5% no signal
100,000-row insertion 448.344 ms 437.220 ms -2.5% no signal
Row fetching in batches of 100 121.860 ms 120.627 ms -1.0% no signal
Row fetching in batches of 10,000 134.171 ms 132.941 ms -2.1% no signal
Repeated positional queries 33.690 ms 33.397 ms -1.7% no signal
Repeated named-parameter queries 36.497 ms 35.522 ms -2.8% no signal
Legacy 100,000-row insertion 346.829 ms 342.302 ms -2.5% no signal
Insertion with explicit input sizes 480.484 ms 502.219 ms +1.1% no signal
Joined aggregation queries 159.818 ms 160.429 ms +0.4% no signal
Large joined-result fetching 178.737 ms 177.359 ms -1.3% no signal
1.2-million-row fetching 3551.903 ms 3521.834 ms -0.7% no signal
Common table expression queries 5.201 ms 5.008 ms -3.7% no signal
256 KiB VARCHAR(MAX) / fetchall() 1.469 ms 1.475 ms +1.5% no signal
10,000 scalar values / fetchval() (debug disabled) 106.996 ms 108.002 ms +2.5% no signal
Build and measurement details

ADO build 181461

PR head: d044464b9d6e3c37bd720d8ce6a9ee9984240576
Base: 666f3cb6d23981bb23cd182ec273df10a7b2c805
Measured merge: 54c012130e8eb1b718c6fe4dce7d057889f68b42

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

@subrata-ms Subrata (subrata-ms) changed the title FEAT: Add preview TVP support for async queries FEAT: Add TVP support for async queries Sep 24, 2026
@github-actions github-actions Bot added the pr-size: medium Moderate update size label Sep 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Guard the optional native dependency test and complete the required PR metadata.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds preview async TVP/type-hint support, iterable executemany, cursor state reporting, and expanded integration tests.

Changes:

  • Adds lazy native TVP and SQL type-hint exports.
  • Supports synchronous iterable batches and failure-result preservation.
  • Documents and tests async cursor/result behavior.
File Summary
tests/​AsyncTest/​test_006_async_execute.py Tests iterable batches, TVPs, and type hints.
tests/​AsyncTest/​test_005_async_cursor.py Tests cursor state and result behavior.
tests/​AsyncTest/​test_004_async_logging.py Updates execution logging expectations.
tests/​AsyncTest/​test_001_async_query_native.py Tests native exports. Finding: moderate, 2 votes—add an importorskip guard for the optional dependency at lines 55 and 65.
mssql_python/​async_query/​async_execute.py Adds iterable batch execution and failure handling.
mssql_python/​async_query/​async_cursor.py Adds cursor state and result-contract documentation.
mssql_python/​async_query/​__init__.py Adds lazy native exports. Finding: nit, 1 vote—replace PR placeholders and empty summary with valid metadata.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/AsyncTest/test_001_async_query_native.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 24, 2026 10:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Fix the indentation error that prevents the native async test module from being collected.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread tests/AsyncTest/test_001_async_query_native.py Outdated
Copilot AI review requested due to automatic review settings September 24, 2026 11:05
@github-actions github-actions Bot added pr-size: large Substantial code update and removed pr-size: medium Moderate update size labels Sep 24, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Fix the indentation error that prevents the native async test module from being collected.

Review effort: Lite
Findings: None

Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 24, 2026 11:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Cancellation handling and version-specific collation test behavior need correction.

Review effort: Lite
Findings: None

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

83%


🎯 Overall Coverage

85%


📈 Total Lines Covered: 9468 out of 11122
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/async_query/init.py (100%)
  • mssql_python/async_query/async_cursor.py (100%)
  • mssql_python/async_query/async_execute.py (50.0%): Missing lines 102-106

Summary

  • Total: 31 lines
  • Missing: 5 lines
  • Coverage: 83%

mssql_python/async_query/async_execute.py

Lines 98-110

   98     iteration_failed = False
   99 
  100     def parameter_rows():
  101         nonlocal iteration_failed
! 102         try:
! 103             yield from seq_of_parameters
! 104         except BaseException:
! 105             iteration_failed = True
! 106             raise
  107 
  108     logger.debug(
  109         "AsyncCursor.executemany: starting; use_prepare=%s",
  110         use_prepare,


📋 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.connection.connection_pool.cpp: 82.3%
mssql_python.pybind.connection.connection.cpp: 83.1%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.async_query.async_execute.py: 91.5%
mssql_python.pybind.fetch_temporal.hpp: 92.1%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Copilot AI review requested due to automatic review settings September 24, 2026 11:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The pinned native core dependency must be updated or the new exports gated before approval.

Review effort: Lite
Findings: None

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The UTF-8 collation test must skip servers that do not support that version-specific collation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread tests/AsyncTest/test_007_async_fetch.py
Skip test if SQL Server does not support the specified collation.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 24, 2026 11:52
@subrata-ms Subrata (subrata-ms) changed the title FEAT: Add TVP support for async queries FEAT: TVP support for async queries Sep 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Correctness depends on external Rust-core behavior and live SQL Server integration that could not be exercised here.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 24, 2026 13:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Native-backed TVP behavior and the extensive live-SQL execution matrix require final human validation.

Review effort: Balanced
Findings: None

Copilot AI review requested due to automatic review settings September 24, 2026 15:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

A new fetch test relies on undefined SQL row ordering and may fail across execution plans or server versions.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread tests/AsyncTest/test_007_async_fetch.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 24, 2026 15:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Correctness depends heavily on external native bindings and live SQL Server behavior requiring final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Copilot AI balanced review requested due to automatic review settings October 8, 2026 10:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Native-backed async behavior and live SQL integration require final human validation despite no confirmed defect.

0 open findings

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

SQL_VECTOR: int

_NATIVE_EXPORTS = {
"_TableValuedParameter": "TableValuedParameter",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #814 - High: This exposes a TVP constructor whose pinned dependency, mssql-python-rs==0.3.0, rejects valid numeric and temporal column metadata. Definitions such as decimal(12,2), numeric(38,10), and temporal scale 3 fail during construction, even for populated or empty tables, because the native builder incorrectly applies scalar-NULL restrictions.

Valid TVPs therefore cannot reach async execution. Please correct native column-template and NULL-cell conversion, pin the corrected dependency, and add regression coverage for populated, NULL-cell, and empty TVPs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: large Substantial code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants