GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-41930: Add support for SQLite serialise/deserialise API by erlend-aasland · Pull Request #26728 · python/cpython

Conversation

erlend-aasland commented Jun 14, 2021 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

https://bugs.python.org/issue41930

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Erlend E. Aasland added 2 commits

June 14, 2021 23:43


          Expose sqlite3_serialize and sqlite3_deserialize

98957b6


          Wrap in ifdef for now

79ecb6c

the-knights-who-say-ni added the CLA signed label

Jun 14, 2021

bedevere-bot added the awaiting review label

Jun 14, 2021

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 6 commits

June 15, 2021 21:50


          Merge branch 'main' into sqlite-serialize


          Merge branch 'main' into sqlite-serialize

d37484e


          Add basic tests

74ab8a7


          Let AC convert data to buffer

125a58c


          Test deserialize type errors

42f8cd5


          Skip tests if serialize API is missing

07d3b5c

erlend-aasland force-pushed the sqlite-serialize branch from 2739573 to 07d3b5c Compare

June 16, 2021 18:50

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

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with hooray emoji

All reactions

🎉 1 reaction

Comment thread

Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_sqlite/connection.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot added awaiting changes and removed awaiting review labels

Feb 28, 2022

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Erlend Egeberg Aasland and others added 3 commits

February 28, 2022 07:16


          Update Doc/library/sqlite3.rst

cea7219

Co-authored-by: Jelle Zijlstra <[email protected]>


          Address review

7ddb4b7

- adjust docs
- deserialize returns None
- remove 'non-standard' from docstring


          Address review: try to avoid memory allocations when serializing

7edccfe

erlend-aasland requested a review from JelleZijlstra

February 28, 2022 10:55

JelleZijlstra approved these changes

Feb 28, 2022

View reviewed changes

JelleZijlstra left a comment

Copy link

Copy Markdown

Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with hooray emoji

All reactions

🎉 1 reaction

bedevere-bot added awaiting merge and removed awaiting changes labels

Feb 28, 2022

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lostnet mentioned this pull request

Mar 2, 2022

sqlite3 "attempt to write a readonly database" with legacy and memory database backend NixOS/nixops#1490

Closed

kumaraditya303 reviewed

Mar 5, 2022

View reviewed changes

Comment thread

PCbuild/_sqlite3.vcxproj Resolved

Uh oh!

There was an error while loading. Please reload this page.

Erlend Egeberg Aasland and others added 2 commits

March 5, 2022 17:09


          Update PCbuild/_sqlite3.vcxproj

1db4093

Co-authored-by: Kumar Aditya <[email protected]>


          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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland commented Apr 5, 2022

Copy link

Copy Markdown

Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Merge branch 'main' into sqlite-serialize

75c3168

JelleZijlstra approved these changes

Apr 5, 2022

View reviewed changes

JelleZijlstra merged commit a755124 into python:main

Apr 5, 2022

bedevere-bot removed the awaiting merge label

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra mentioned this pull request

Apr 15, 2022

sqlite3: add 3.11 additions python/typeshed#7625

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

test_sqlite3.test_dbapi: test_deserialize_too_much_data_64bit uses undefined size (and requires 24 Exabytes of RAM) #93117

Closed

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

JelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

erlend-aasland commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Feb 28, 2022

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Feb 28, 2022

Uh oh!

Uh oh!

erlend-aasland commented Mar 8, 2022

Uh oh!

erlend-aasland commented Apr 5, 2022

Uh oh!

JelleZijlstra commented Apr 5, 2022

Uh oh!

erlend-aasland commented Apr 5, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

erlend-aasland commented Jun 14, 2021 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.