FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

FIX: Handle Row objects in executemany DAE fallback path (GH-629) by gargsaumya · Pull Request #630 · microsoft/mssql-python · GitHub

FIX: Handle Row objects in executemany DAE fallback path (GH-629) - #630

Merged
gargsaumya merged 8 commits into
mainfrom
saumya/gh-629
Jun 11, 2026
Merged

FIX: Handle Row objects in executemany DAE fallback path (GH-629)#630
gargsaumya merged 8 commits into
mainfrom
saumya/gh-629

Conversation

gargsaumya commented Jun 10, 2026
edited by azure-boards Bot
Loading

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#45679

GitHub Issue: #629


Summary

This pull request introduces a compatibility improvement to the executemany method in mssql_python/cursor.py. It ensures that when iterating through rows of parameters, any Row objects are converted to tuples before execution. This conversion is necessary because the internal _map_sql_type logic only recognizes primitive types, and not Row objects, which prevents potential type errors during execution.

Parameter compatibility improvement:

  • Updated the executemany method to convert Row objects to tuples before passing them to execute, ensuring compatibility with the internal type mapping logic.

When executemany detects DAE parameters (large strings >4000 chars for
varchar(max) columns), it falls back to row-by-row execution. This
fallback was passing Row objects directly to execute(), but _map_sql_type()
only recognizes primitive types (str, int, etc.), causing TypeError.

Fix: Convert Row objects to tuples before passing to execute() in the
DAE fallback path. This preserves compatibility while allowing Row objects
from fetch APIs to be used directly with executemany.

Root cause: This bug was introduced when Row objects were added in v0.10.0
(commit 54b649b, PR #75, June 2025) and fetch APIs changed from returning
tuples to returning Row objects. The DAE fallback path was not updated to
handle the new Row type.

Fixes #629
Copilot AI review requested due to automatic review settings June 10, 2026 06:14
github-actions Bot added pr-size: small Minimal code update labels Jun 10, 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull request overview

This PR fixes a compatibility issue in Cursor.executemany() when it falls back to row-by-row execution for DAE (streaming) parameters: it ensures fetched Row objects can be used as parameter rows by converting them into tuples before calling execute().

Changes:

  • Convert Row instances to tuples in the DAE fallback loop inside executemany().
  • Add inline commentary explaining why the conversion is needed for execute() compatibility.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mssql_python/cursor.py Outdated
Comment thread mssql_python/cursor.py

github-actions Bot commented Jun 10, 2026
edited
Loading

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

80%


📈 Total Lines Covered: 6691 out of 8291
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/cursor.py (100%)

Summary

  • Total: 2 lines
  • Missing: 0 lines
  • Coverage: 100%

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.3%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.pybind.connection.connection.cpp: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Addresses review comments:
- Use isinstance(row, Row) for precise type checking
- Update comment to accurately explain execute() unwrapping behavior

When executemany() detects DAE parameters (VARCHAR(MAX) >4000 chars),
it falls back to row-by-row execution. This fix converts Row objects
to tuples before passing to execute(), since execute() only unwraps
tuple/list/dict arguments, not Row objects.

Includes regression test to prevent future issues.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

requesting changes, there's an underlying day 0 bug here, added details.
happy to re-review once the unwrap moves into execute().

Comment thread mssql_python/cursor.py Outdated
Comment thread tests/test_004_cursor.py Outdated
execute() only unwrapped tuple/list/dict single args, so a Row (e.g. from
fetchone()) was treated as a single scalar parameter, raising
'Unsupported parameter type'. Normalize a single Row arg to a tuple at the
unwrap site so both execute(sql, row) and the executemany DAE fallback work.
Removes the now-redundant Row->tuple pre-patch in the DAE path. Test uses
isinstance(rows[0], mssql_python.Row) instead of fragile name check.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm, thanks
approving with a regression test suggestion

Comment thread tests/test_004_cursor.py

Copy link
Copy Markdown
Contributor

looks good to me.. Approved with a minor change to add one additional test

The GH-629 fix lives in execute()'s single-arg unwrap, but the existing
test only exercised the executemany DAE path. Add a test that passes a Row
directly to execute(sql, row) for both the regular bind path and the
VARCHAR(MAX) DAE path, guarding the fix surface against future refactors.
github-actions Bot added pr-size: medium Moderate update size and removed pr-size: small Minimal code update labels Jun 11, 2026
gargsaumya merged commit 77bc865 into main Jun 11, 2026
29 of 30 checks passed
gargsaumya mentioned this pull request Jun 12, 2026
gargsaumya added a commit that referenced this pull request Jun 12, 2026
[AB#45750](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/45750)

Release mssql-python v1.9.0. Bumps `__version__`, `setup.py` version,
and updates `PyPI_Description.md` with this release's customer-facing
changes.

### Summary

#### Enhancements
- **Row Objects in Bulk Copy** - `bulkcopy` now accepts `Row` objects
(and lists) directly, converting each row to a tuple automatically
(#615).
- **Cached Parameter Type Resolution for NULLs** - `SQLDescribeParam`
results are cached per statement when binding `NULL` parameters,
removing redundant server round-trips and fixing incorrect type
fallbacks for all-NULL columns and VARBINARY types (#614).

#### Bug Fixes
- **macOS / Linux Import Failure** - simdutf is now always statically
linked via FetchContent, fixing import/symbol failures on machines
without simdutf at the CI build path (#608). Resolves #607, #628.
- **executemany Large Decimal Handling** - Fixed a `SQL_C_NUMERIC` type
mismatch when inserting `Decimal` values outside the SQL Server `MONEY`
range via `executemany` (#611). Resolves #609.
- **Exception Pickling** - DB-API exception subclasses and
`ConnectionStringParseError` now implement `__reduce__` for correct
pickle/unpickle round-trips (#616). Resolves #587.
- **PRINT Messages in nextset()** - Diagnostic messages from subsequent
result sets are captured on `SQL_SUCCESS_WITH_INFO` during `nextset()`
(#618). Resolves #612.
- **Row Objects in executemany DAE Path** - `executemany` converts `Row`
objects to tuples in the DAE fallback path, fixing `varchar(max)` writes
(#630). Resolves #629.
- **Static Type-Checking of Fetch Methods** -
`fetchone`/`fetchmany`/`fetchall` are no longer reassigned as instance
attributes, fixing type-checking under `ty` (#631). Resolves #620.

#### Version bump
- `mssql_python/__init__.py`: `__version__` 1.8.0 → 1.9.0
- `setup.py`: version 1.8.0 → 1.9.0
- `PyPI_Description.md`: updated "What's new" section to v1.9.0

Bundled `mssql_py_core` version unchanged (0.1.4).
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

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL