Skip to content

FIX: Capture PRINT messages in nextset() via SQL_SUCCESS_WITH_INFO#618

Merged
jahnvi480 merged 5 commits into
mainfrom
jahnvi/fix-nextset-messages-612
Jun 8, 2026
Merged

FIX: Capture PRINT messages in nextset() via SQL_SUCCESS_WITH_INFO#618
jahnvi480 merged 5 commits into
mainfrom
jahnvi/fix-nextset-messages-612

Conversation

@jahnvi480

@jahnvi480 jahnvi480 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#45395

GitHub Issue: #612


Summary

This pull request addresses an issue where diagnostic messages (such as SQL Server PRINT output) from subsequent result sets were not properly captured when using the nextset() method. The fix ensures that all messages are collected, even across multiple result sets, and adds comprehensive tests to verify this behavior.

Bug Fix: Diagnostic Message Handling

  • Updated the nextset() method in cursor.py to capture diagnostic messages (e.g., PRINT output) when SQL returns SQL_SUCCESS_WITH_INFO, ensuring messages from all result sets are preserved.

Testing Improvements

  • Added several new tests in test_004_cursor.py to verify that PRINT messages are correctly captured across multiple result sets and that messages from previous result sets are cleared appropriately:
    • Test for multiple PRINT statements across result sets via nextset()
    • Test for PRINT messages interleaved with SELECT statements
    • Test for three consecutive PRINT messages across nextset() calls
    • Test to ensure nextset() clears messages from the previous result set

Copilot AI review requested due to automatic review settings June 2, 2026 14:42
@github-actions github-actions Bot added the pr-size: medium Moderate update size label Jun 2, 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.

Pull request overview

This PR fixes a gap in diagnostic message handling when iterating across multiple result sets: Cursor.nextset() now collects diagnostics (e.g., SQL Server PRINT output) when SQLMoreResults returns SQL_SUCCESS_WITH_INFO, ensuring messages from subsequent result sets aren’t silently dropped.

Changes:

  • Update Cursor.nextset() to pull diagnostic records on SQL_SUCCESS_WITH_INFO from SQLMoreResults.
  • Add regression and scenario tests validating PRINT message capture across multiple nextset() calls (including mixed PRINT + SELECT).
  • Add a test asserting nextset() clears prior result-set messages.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
mssql_python/cursor.py Capture diagnostics on SQL_SUCCESS_WITH_INFO during nextset() to preserve PRINT output across result sets.
tests/test_004_cursor.py Add regression tests covering multi-result PRINT diagnostics behavior through nextset().

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

…H-612)

nextset() called DDBCSQLMoreResults but never checked for
SQL_SUCCESS_WITH_INFO, so diagnostic messages (e.g. PRINT output)
from subsequent result sets were silently lost.

Add the same SQL_SUCCESS_WITH_INFO -> DDBCSQLGetAllDiagRecords
pattern already used in execute().

Tests added:
- test_cursor_messages_nextset_multiple_prints (exact repro from issue)
- test_cursor_messages_nextset_print_with_select (mixed PRINT + SELECT)
- test_cursor_messages_nextset_three_prints (3 consecutive PRINTs)
- test_cursor_messages_nextset_clears_previous (clearing semantics)

Closes #612
@jahnvi480 jahnvi480 force-pushed the jahnvi/fix-nextset-messages-612 branch from 31d4f4d to 6e4f388 Compare June 2, 2026 14:47
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

80%


📈 Total Lines Covered: 6670 out of 8271
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/cursor.py (100%)

Summary

  • Total: 7 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.7%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.1%
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

@KellyKr

KellyKr commented Jun 3, 2026

Copy link
Copy Markdown

I just ran into this and see the fix will be coming. Nice!

To ask for a bit more: the type of cursor.messages is list[Unknown], it would be nice to have correct typing on it.

Comment thread mssql_python/cursor.py Outdated
Comment thread mssql_python/cursor.py Outdated
Comment thread tests/test_004_cursor.py Outdated
Comment thread tests/test_004_cursor.py
Comment thread tests/test_004_cursor.py
Comment thread mssql_python/cursor.py Outdated
…H-612)

nextset() called DDBCSQLMoreResults but never captured diagnostic
messages, so PRINT output from subsequent result sets was silently
lost.

Changes:
- Add _capture_diagnostics() helper to centralise the pattern of
  collecting diagnostic records on SQL_SUCCESS_WITH_INFO and
  SQL_NO_DATA (trailing PRINT after the final result set).
- Use the helper in execute() and nextset().
- Restore nextset() return type to Optional[bool] to keep the door
  open for future PEP-249 alignment (True/None). Current behaviour
  remains True/False for backward compatibility.

Tests added:
- test_cursor_messages_nextset_multiple_prints (exact repro from issue)
- test_cursor_messages_nextset_print_with_select (mixed PRINT + SELECT,
  verifies only nextset-captured messages)
- test_cursor_messages_nextset_three_prints (3 consecutive PRINTs)
- test_cursor_messages_nextset_clears_previous (clearing semantics)
- test_cursor_messages_nextset_trailing_print (SQL_NO_DATA diagnostics)

Closes #612
@sumitmsft

Copy link
Copy Markdown
Contributor

I just ran into this and see the fix will be coming. Nice!

To ask for a bit more: the type of cursor.messages is list[Unknown], it would be nice to have correct typing on it.

@jahnvi480 While we're touching the messages handling, can we also fix the typing? Today self.messages = [] is inferred as list[Unknown] by IDEs and type-checkers.

The C++ binding (SQLGetAllDiagRecords) always returns Tuple[str, str] per entry. (sql_state_with_error_code, message_text) and the tests already assert this shape. So. do you think it's a safe fix?

The C++ binding (DDBCSQLGetAllDiagRecords) always returns
(sql_state_with_error_code, message_text) tuples.  Without the
annotation, IDEs infer list[Unknown].  Existing tests already
assert this shape.
@jahnvi480

Copy link
Copy Markdown
Contributor Author

Thanks @KellyKr and @sumitmsft for detailed review, I have verified and updated the typing for messages.

@jahnvi480 jahnvi480 requested a review from sumitmsft June 5, 2026 14:41
@jahnvi480 jahnvi480 merged commit 70be653 into main Jun 8, 2026
29 of 30 checks passed
@gargsaumya 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).
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.

5 participants