Skip to content

Conversation

jahnvi480
Copy link
Contributor

@jahnvi480 jahnvi480 commented Aug 14, 2025

Work Item / Issue Reference

AB#34924


Summary

This pull request introduces a new convenience method, skip, to the Cursor class in mssql_python/cursor.py, which allows users to advance the cursor position by a specified number of rows without fetching them. Comprehensive tests have been added to validate the method's behavior, including edge cases and integration with existing fetch methods.

New feature: Cursor skipping

  • Added skip(count: int) method to the Cursor class, enabling users to efficiently advance the cursor by a given number of rows without returning those rows. The method checks for closed cursors, validates arguments, supports no-op for zero, and raises appropriate errors for invalid usage.

Testing and validation

  • Added test_cursor_skip_basic_functionality to verify that skip advances the cursor as expected and integrates correctly with fetchone.
  • Added tests for edge cases: skipping zero rows (test_cursor_skip_zero_is_noop), empty result sets (test_cursor_skip_empty_result_set), skipping past the end (test_cursor_skip_past_end), invalid arguments (test_cursor_skip_invalid_arguments), and closed cursors (test_cursor_skip_closed_cursor).
  • Added integration tests to ensure skip works correctly with fetchone, fetchmany, and fetchall methods (test_cursor_skip_integration_with_fetch_methods).

@github-actions github-actions bot added the pr-size: medium Moderate update size label Aug 14, 2025
Copy link
Contributor

@gargsaumya gargsaumya left a comment

Choose a reason for hiding this comment

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

Since skip() is just an alias for scroll(count, 'relative'), it carries the same inefficiency concerns for large skips. Approving for now.

Copy link
Contributor

@sumitmsft sumitmsft left a comment

Choose a reason for hiding this comment

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

Please make the changes to the logic of PR# 180 as this PR depends on it. Let's hold on to it for now.

@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Aug 25, 2025
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: medium Moderate update size labels Aug 25, 2025
@jahnvi480 jahnvi480 requested a review from sumitmsft August 25, 2025 09:50
@jahnvi480
Copy link
Contributor Author

@sumitmsft @gargsaumya Resolved comments in #180 Please have a look at it and let me know if there are any other changes required.

Copy link
Contributor

@sumitmsft sumitmsft left a comment

Choose a reason for hiding this comment

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

Straightforward implementation as it is dependent on PR#180

### Work Item / Issue Reference  
<!-- 
IMPORTANT: Please follow the PR template guidelines below.
For mssql-python maintainers: Insert your ADO Work Item ID below (e.g.
AB#37452)
For external contributors: Insert Github Issue number below (e.g. #149)
Only one reference is required - either GitHub issue OR ADO Work Item.
-->

<!-- mssql-python maintainers: ADO Work Item -->
>
[AB#34893](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/34893)

-------------------------------------------------------------------
### Summary   
This pull request adds comprehensive support for capturing and managing
diagnostic messages (such as SQL Server PRINT statements and warnings)
in the `mssql_python` driver's `Cursor` class, following the DBAPI
specification. It introduces a new `messages` attribute on the cursor,
ensures messages are cleared or preserved at the correct times, and
provides robust testing for these behaviors. Additionally, it implements
the underlying C++ binding for retrieving all diagnostic records from
the ODBC driver.

**Diagnostic message handling improvements:**

* Added a `messages` attribute to the `Cursor` class to store diagnostic
messages, and ensured it is cleared before each non-fetch operation
(e.g., `execute`, `executemany`, `close`, `commit`, `rollback`,
`scroll`, and `nextset`) to comply with DBAPI expectations.
(`mssql_python/cursor.py`)
* After each statement execution and fetch operation, diagnostic
messages (including informational and warning messages) are collected
and appended to the `messages` list, using a new C++ binding.
(`mssql_python/cursor.py`, `mssql_python/pybind/ddbc_bindings.cpp`)

**Native driver and binding enhancements:**

* Implemented the `SQLGetAllDiagRecords` function in the C++ pybind
layer to retrieve all diagnostic records from an ODBC statement handle,
handling both Windows and Unix platforms, and exposed it as
`DDBCSQLGetAllDiagRecords` to Python.
(`mssql_python/pybind/ddbc_bindings.cpp`)

**Testing and specification compliance:**

* Added a comprehensive test suite to verify message capturing,
clearing, preservation across fetches, handling of multiple messages,
message formatting, warning capture, manual clearing, and error
scenarios, ensuring compliance with DBAPI and robust behavior.
(`tests/test_004_cursor.py`)

**Other cursor improvements:**

* Refactored the `skip` method to validate arguments more strictly,
clear messages before skipping, and improve error handling and
documentation. (`mssql_python/cursor.py`)

These changes significantly improve the usability and correctness of
message handling in the driver, making it easier for users to access and
manage SQL Server informational and warning messages in Python
applications.

---------

Co-authored-by: Jahnvi Thakkar <jathakkar@microsoft.com>
@github-actions github-actions bot added pr-size: large Substantial code update and removed pr-size: medium Moderate update size labels Aug 27, 2025
@jahnvi480 jahnvi480 merged commit 4f56411 into jahnvi/cursor_scroll Aug 27, 2025
3 checks passed
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