Skip to content

bpo-41930: Add support for SQLite serialise/deserialise API#26728

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.

Sign up for GitHub

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

Jump to bottom
Merged
JelleZijlstra merged 73 commits into from
Apr 5, 2022
Merged

bpo-41930: Add support for SQLite serialise/deserialise API #26728

JelleZijlstra merged 73 commits into from
Apr 5, 2022
+435 −1

Conversation

@erlend-aasland
Copy link
Contributor

@erlend-aaslanderlend-aasland commented Jun 14, 2021•
edited
Loading

@erlend-aaslanderlend-aasland changed the title bpo-41930: Add support for SQLite serialise/deserialise API[WIP] bpo-41930: Add support for SQLite serialise/deserialise APIJun 14, 2021
Erlend E. Aasland added 18 commits June 16, 2021 23:31
Only reset pending statements if needed
6c5147d
Test deserialize with corrupt data
d24fc61
Cast sqlite3_deserialize() arguments
7db9dfb
Refactor tests
527ee92
Check thread and connection when deserializing
6348eaa
Simplify serialize return value
6e4344f
Don't return size; use len() on the returned bytes object instead.
Reset cursors and statements before deserialize
bde320b
Add basic test for this
Add NEWS stub
3d8c642
Merge branch 'main' into sqlite-serialize
c28f2bc
Merge branch 'main' into sqlite-serialize
ddf4da1
Merge branch 'main' into sqlite-serialize
1bdb611
Regen clinic after pythonGH-27155.
Merge branch 'main' into sqlite-serialize
d6934f9
Sync with main bco. pythonGH-27273
Free SQLite buffer after Py conversion
fac7cdc
Transfer ownership of the deserialized data to SQLite
9df648f
Add docstrings
02d0e45
Merge branch 'main' into sqlite-serialize
a22bb3d
Sync with main bco. pythonGH-27431
Add What's New entry
bf5ab3d
Add Docs
5a7f974
JelleZijlstra
JelleZijlstra requested changes Feb 28, 2022
View reviewed changes
Copy link
Member

@JelleZijlstraJelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks. I'm on board with merging this, since it's generally useful for sqlite3 to expose more of the underlying SQLite API, the feature is simple and shouldn't need much maintenance in the future, and there haven't been any objections on the issue.

I do have a few small comments. Once those are addressed, I'll ask Guido for a second look and then merge the PR.

@bedevere-bot
Copy link

bedevere-bot commented Feb 28, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Erlend Egeberg Aaslandand others added 3 commits February 28, 2022 07:16
@JelleZijlstra
Update Doc/library/sqlite3.rst
cea7219
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Address review
7ddb4b7
- adjust docs - deserialize returns None - remove 'non-standard' from docstring
Address review: try to avoid memory allocations when serializing
7edccfe
JelleZijlstra
JelleZijlstra approved these changes Feb 28, 2022
View reviewed changes
Copy link
Member

@JelleZijlstraJelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good! @gvanrossum I'm planning to merge this PR (adding two new functions to sqlite3)

@gvanrossum
Copy link
Member

gvanrossum commented Feb 28, 2022

Looks good! @gvanrossum I'm planning to merge this PR (adding two new functions to sqlite3)

SGTM

@lostnetlostnet mentioned this pull request Mar 2, 2022
Closed
kumaraditya303
kumaraditya303 reviewed Mar 5, 2022
View reviewed changes
Erlend Egeberg Aaslandand others added 2 commits March 5, 2022 17:09
@kumaraditya303
Update PCbuild/_sqlite3.vcxproj
1db4093
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Merge branch 'main' and regen clinic
03b94b0
@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Mar 8, 2022

FYI: synced with main and regenerated clinic bco. GH-31612.

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Apr 5, 2022

@JelleZijlstra, will you be able to merge this before todays alpha?

@JelleZijlstraJelleZijlstra self-assigned this Apr 5, 2022
@JelleZijlstra
Copy link
Member

JelleZijlstra commented Apr 5, 2022

Oops, sorry for dropping this. There are merge conflicts on the configure scripts, could you resolve those?

@JelleZijlstraJelleZijlstra merged commit a755124 into python:mainApr 5, 2022
@erlend-aaslanderlend-aasland deleted the sqlite-serialize branch April 5, 2022 16:24
@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Apr 5, 2022

Thanks, Jelle!

@JelleZijlstraJelleZijlstra mentioned this pull request Apr 15, 2022
Merged
JelleZijlstra added a commit to JelleZijlstra/typeshed that referenced this pull request Apr 15, 2022
@JelleZijlstra
sqlite3: add 3.11 additions
0ba23bb
- Blob from python/cpython#30680 (and anticipating that python/cpython#91550 will be merged) - Aggregate window functions from python/cpython#20903 - Serialize/deserialize from python/cpython#26728 - Limit setting from python/cpython#28463
srittau pushed a commit to python/typeshed that referenced this pull request Apr 16, 2022
@JelleZijlstra
sqlite3: add 3.11 additions (#7625)
d802e65
- Blob from python/cpython#30680 (and anticipating that python/cpython#91550 will be merged) - Aggregate window functions from python/cpython#20903 - Serialize/deserialize from python/cpython#26728 - Limit setting from python/cpython#28463
@The-CompilerThe-Compiler mentioned this pull request May 23, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303kumaraditya303kumaraditya303 left review comments

@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

@berkerpeksagberkerpeksagAwaiting requested review from berkerpeksagberkerpeksag is a code owner

@serhiy-storchakaserhiy-storchakaAwaiting requested review from serhiy-storchaka

@pablogsalpablogsalAwaiting requested review from pablogsal

Assignees

@JelleZijlstraJelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@erlend-aasland@bedevere-bot@gvanrossum@JelleZijlstra@kumaraditya303@pablogsal@the-knights-who-say-ni