Skip to content
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

gh-118221: Always use the default row factory in sqlite3.iterdump() #118223

Merged

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Apr 24, 2024

The iterdump() implementation depends on the row factory returning
resulting rows as tuples; it will fail with custom row factories like
for example a dict factory.

Fixed by overriding the row factory of the cursor used by iterdump().
FTR, this does not affect the row factory of the parent connection.

…mp()

The iterdump() implementation depends on the row factory returning
resulting rows as tuples; it will fail with custom row factories like
for example a dict factory.

Fixed by overriding the row factory of the cursor used by iterdump().
FTR, this does not affect the row factory of the parent connection.
@erlend-aasland
Copy link
Contributor Author

@kesry, can you see if this patch works for you?

@erlend-aasland
Copy link
Contributor Author

cc. @felixxm who has poked around in the iterdump() implementation lately.

Copy link
Contributor

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@erlend-aasland Thanks 👍

Lib/test/test_sqlite3/test_dump.py Outdated Show resolved Hide resolved
Lib/test/test_sqlite3/test_dump.py Show resolved Hide resolved
Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
@erlend-aasland
Copy link
Contributor Author

Thanks for the review, Mariusz! I'll land this later this week, in case anyone else wants to take a look.

@erlend-aasland erlend-aasland self-assigned this Apr 24, 2024
Lib/test/test_sqlite3/test_dump.py Outdated Show resolved Hide resolved
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@kesry
Copy link

kesry commented Apr 25, 2024

cc. @felixxm who has poked around in the iterdump() implementation lately.

That's good, thanks!

@erlend-aasland
Copy link
Contributor Author

Thanks for the review, Serhiy!

@erlend-aasland erlend-aasland merged commit e38b43c into python:main Apr 25, 2024
33 checks passed
@erlend-aasland erlend-aasland deleted the sqlite/iterdump-custom-row-factory branch April 25, 2024 08:11
@miss-islington-app

This comment was marked as outdated.

@miss-islington-app

This comment was marked as outdated.

@bedevere-app
Copy link

bedevere-app bot commented Apr 25, 2024

GH-118270 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Apr 25, 2024
erlend-aasland added a commit that referenced this pull request Apr 25, 2024
…ump() (#118223) (#118270)

sqlite3.iterdump() depends on the row factory returning resulting rows
as tuples; it will fail with custom row factories like for example a
dict factory.

With this commit, we explicitly reset the row factory of the cursor used
by iterdump(), so we always get predictable results. This does not
affect the row factory of the parent connection.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-bot

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants