SNOW-3264599: Python - implement cursor reset method#723
Merged
sfc-gh-asolarski merged 1 commit intomainfrom Mar 27, 2026
Merged
Conversation
This was referenced Mar 25, 2026
Contributor
Author
b2a1f1b to
3fc02a0
Compare
0f7a79e to
0dcaa5e
Compare
3fc02a0 to
d1c0313
Compare
0dcaa5e to
9d678e8
Compare
9d678e8 to
6ed1405
Compare
python/tests/integ/test_cursor.py
Outdated
| # Then it should be None (per PEP 249) | ||
| assert result is None | ||
| # Then it should be -1 (per PEP 249) | ||
| assert result == -1 |
Contributor
There was a problem hiding this comment.
Does it pass with reference driver?
Contributor
Author
There was a problem hiding this comment.
fixed (it seems that None incorrect per pep249 but backward compatible)
I covered this additionally with more integration tests.
6ed1405 to
6739317
Compare
Base automatically changed from
03-20-snow-3243341_python_wrapper_-_support_pandas_and_pyarrow_fetching
to
main
March 26, 2026 15:11
6739317 to
84c72e0
Compare
|
|
||
|
|
||
| @contextmanager | ||
| def create_statement(connection: Connection, query: str) -> Generator[StatementHandle]: |
Contributor
There was a problem hiding this comment.
Suggested change
| def create_statement(connection: Connection, query: str) -> Generator[StatementHandle]: | |
| Query = str | |
| def create_statement(connection: Connection, query: Query) -> Generator[StatementHandle]: |
Nit, we can introduce Query = str it would be aligned with Row = tuple[Any, ...] DictRow = dict[str, Any]. Possibly having snowflake.connector.types can be reasonable place to keep all custom types. Not mandatory in this PR
Contributor
Author
There was a problem hiding this comment.
It looks like a good idea indeed.
I will take a look into that in one of the following PRs.
84c72e0 to
b717b89
Compare
d3f0ab1 to
3f48dcf
Compare
sfc-gh-turbaszek
approved these changes
Mar 27, 2026
3f48dcf to
06aa4bd
Compare
# Conflicts: # python/src/snowflake/connector/cursor.py
06aa4bd to
bb36ffa
Compare
This was referenced Mar 27, 2026
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

TL;DR
Implemented proper cursor state management by adding a
reset()method and updatingclose()to properly clean up resources and return status indicators.What changed?
reset()method: Clears all result-related state includingexecute_result,_iterator,_description,_sqlstate,_fetch_mode,_binding_data, and_rownumber. Theclosingparameter preserves_rowcountwhen set toTrue.close()method: Now callsreset(closing=True), clears the messages list, and returns a boolean indicating success (True), already closed (False), or exception (None).execute()method: Added_do_resetparameter (defaults toTrue) and moved the reset logic to callreset()at the beginning instead of manually clearing state afterward.executemany()method: Now callsreset()once before the loop for client-side binding scenarios and passes_do_reset=Falseto individualexecute()calls to avoid redundant resets.Why make this change?
This change provides proper cursor lifecycle management by centralizing state cleanup logic in the
reset()method. It ensures consistent behavior across different execution patterns, prevents state leakage between queries, and provides better error handling and status reporting for cursor operations. Theclose()method now properly indicates its success status, andexecutemany()avoids unnecessary resets between individual executions while maintaining clean state.