FIX: Handle Row objects in executemany DAE fallback path (GH-629)#630
Conversation
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
There was a problem hiding this comment.
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
Rowinstances to tuples in the DAE fallback loop insideexecutemany(). - 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.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
📋 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
|
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.
bewithgaurav
left a comment
There was a problem hiding this comment.
requesting changes, there's an underlying day 0 bug here, added details.
happy to re-review once the unwrap moves into execute().
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.
bewithgaurav
left a comment
There was a problem hiding this comment.
lgtm, thanks
approving with a regression test suggestion
|
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.
[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).
Work Item / Issue Reference
Summary
This pull request introduces a compatibility improvement to the
executemanymethod inmssql_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_typelogic only recognizes primitive types, and not Row objects, which prevents potential type errors during execution.Parameter compatibility improvement:
executemanymethod to convert Row objects to tuples before passing them toexecute, ensuring compatibility with the internal type mapping logic.