FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-41930: Add support for SQLite serialise/deserialise API by erlend-aasland · Pull Request #26728 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

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

Merged
JelleZijlstra merged 73 commits into
python:mainfrom
erlend-aasland:sqlite-serialize
Apr 5, 2022
ConversationCommits (73)ChecksFiles changed
Merged

bpo-41930: Add support for SQLite serialise/deserialise API#26728
JelleZijlstra merged 73 commits into
python:mainfrom
erlend-aasland:sqlite-serialize

Conversation

erlend-aasland commented Jun 14, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

erlend-aasland changed the title bpo-41930: Add support for SQLite serialise/deserialise API [WIP] bpo-41930: Add support for SQLite serialise/deserialise API Jun 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 requested changes Feb 28, 2022
View reviewed changes

JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread Modules/_sqlite/connection.c
Comment thread Modules/_sqlite/connection.c Outdated
Comment thread Modules/_sqlite/connection.c
Comment thread Modules/_sqlite/connection.c Outdated
Comment thread Doc/library/sqlite3.rst Outdated
Comment thread Doc/library/sqlite3.rst Outdated

bedevere-bot commented Feb 28, 2022

Copy link
Copy Markdown

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 Aasland and others added 3 commits February 28, 2022 07:16
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 approved these changes Feb 28, 2022
View reviewed changes

JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

gvanrossum commented Feb 28, 2022

Copy link
Copy Markdown
Member

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

SGTM

lostnet mentioned this pull request Mar 2, 2022
Closed
kumaraditya303 reviewed Mar 5, 2022
View reviewed changes
Comment thread PCbuild/_sqlite3.vcxproj Outdated
Erlend Egeberg Aasland and others added 2 commits March 5, 2022 17:09
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 commented Mar 8, 2022

Copy link
Copy Markdown
Contributor Author

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

erlend-aasland commented Apr 5, 2022

Copy link
Copy Markdown
Contributor Author

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

JelleZijlstra self-assigned this Apr 5, 2022

JelleZijlstra commented Apr 5, 2022

Copy link
Copy Markdown
Member

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

JelleZijlstra merged commit a755124 into python:main Apr 5, 2022
erlend-aasland deleted the sqlite-serialize branch April 5, 2022 16:24

erlend-aasland commented Apr 5, 2022

Copy link
Copy Markdown
Contributor Author

Thanks, Jelle!

JelleZijlstra mentioned this pull request Apr 15, 2022
Merged
JelleZijlstra added a commit to JelleZijlstra/typeshed that referenced this pull request Apr 15, 2022
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
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-Compiler mentioned this pull request May 23, 2022
Closed
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kumaraditya303 kumaraditya303 left review comments
JelleZijlstra JelleZijlstra approved these changes
berkerpeksag Awaiting requested review from berkerpeksag
serhiy-storchaka Awaiting requested review from serhiy-storchaka
pablogsal Awaiting requested review from pablogsal

Assignees

JelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL