-
Notifications
You must be signed in to change notification settings - Fork 7
FIX: Allowing access using column name and index #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
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 improves database query result handling by introducing named tuple support in fetchone(), enhancing error handling in the C++ bindings, and refactoring the test framework for better maintainability.
- Enhanced fetchone() to support attribute access via named tuples.
- Improved error and logging behavior in the C++ bindings.
- Refactored test cases to use a helper function for table cleanup and row value comparisons.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/test_004_cursor.py | Refactored tests to use drop_table_if_exists and compare_row_value for clearer assertions and cleanup. |
mssql_python/pybind/ddbc_bindings.cpp | Updated FetchOne_wrap with better error handling and corrected docstring for parameter usage. |
mssql_python/cursor.py | Modified fetchone to return a namedtuple when possible, enabling both index and attribute access. |
main.py | Added debug prints for row type and content to help trace fetchone results. |
Comments suppressed due to low confidence (1)
mssql_python/pybind/ddbc_bindings.cpp:1810
- The parameter name in this docstring should be updated to 'row_list' to match the function signature for clarity.
// @param row: A Python object reference that will be populated with a named tuple containing the fetched row data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR follows good practices, is secure, and robustly upgrades the user-facing API. With minor refactoring for efficiency and logging, it would be even better. No memory leaks, deadlocks, or security issues identified. Logic is correct, and changes are well-tested.
This pull request introduces several enhancements to the database interaction logic and improves the testing framework for SQL data types. The most significant changes include adding support for named tuples in
fetchone
results, improving error handling in the C++ bindings, and refactoring test cases for better cleanup and assertion practices.Enhancements to
fetchone
functionality:mssql_python/cursor.py
: Modified thefetchone
method to return a named tuple when valid field names are available, enabling attribute-based access to query results. Falls back to a regular tuple if named tuple creation fails.mssql_python/cursor.py
: Addedcollections
import to support named tuple creation.Improvements to C++ bindings:
mssql_python/pybind/ddbc_bindings.cpp
: UpdatedFetchOne_wrap
to handle named tuples on the Python side, added error handling for column count retrieval, and improved logging for debugging.Refactoring and cleanup in test cases:
tests/test_004_cursor.py
: Refactored multiple test cases to use thedrop_table_if_exists
helper function for cleanup, ensuring tables are dropped before and after tests. [1] [2] [3]tests/test_004_cursor.py
: Replaced direct tuple comparisons in assertions with thecompare_row_value
helper function for improved readability and flexibility. [1] [2] [3]Debugging and logging enhancements:
main.py
: Added debug statements to print the type and content of rows, and safely handle cases whererows
isNone
.