GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-69093: Support basic incremental I/O to blobs in `sqlite3` by erlend-aasland · Pull Request #30680 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Jan 19, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Authored-by: Aviv Palivoda [email protected]
Co-authored-by: Erlend E. Aasland [email protected]

gh-69093

Sorry, something went wrong.

Uh oh!

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

All reactions
palaviv and others added 30 commits July 29, 2020 19:37
@palaviv
support BLOB incremental I/O in sqlite module
4fa0e91
@palaviv
Note that blob size cannot be changed using the blob object
3fe9108
@palaviv
Use assertRaises in tests
865c1c8
@palaviv
Fix doc error
788fd54
@palaviv
blob support sequence protocol
a1361e5
@palaviv
Calculate blob length once at creation
5aedfba
@palaviv
Add initial Doc for sequence protocol
db6ef32
@palaviv
Don't support blob operation
219f4cb
@palaviv
move news entry to blurb
6dafe0e
@palaviv
Add blob to PCBuild
ffac901
@palaviv
Update version
3475fa1
@palaviv
Fix memory leak
f6015ff
@palaviv
Update version
01e526c
@palaviv
Fix CR comments in documentation and testing
354bebf
@palaviv
Fix CR comments in code
9709456
@palaviv
Make readonly and dbname keyword arguements only
e6e5099
@palaviv
Fix more CR comments
e9a8080
@palaviv
Add more indicative error on write bigger then blob length
d4fb1b5
Erlend E. Aasland
Merge branch 'main' into sqlite-blob
a7288f9
Erlend E. Aasland
Adapt sqlite3.Connection.open_blob() to AC
525a9c3
Erlend E. Aasland
Adapt sqlite.Blob to AC
2f65cc8
Erlend E. Aasland
PEP 7 and normalised naming
0dd047f
Erlend E. Aasland
Use Py_NewRef and Py_IsNone
d34a77b
Erlend E. Aasland
Harden blob_open()
9d55705
Erlend E. Aasland
Move blob init to blob_open()
d47ea17
Erlend E. Aasland
initialise rc in blob_ass_subscript()
e07a116
Erlend E. Aasland
Improve blob.seek() parameter naming
982c812
Erlend E. Aasland
Adapt to heap types and allow calling close multiple times
e92495b
Erlend E. Aasland
Consolidate tests
8f2ce8a
Erlend E. Aasland
Naming: read_length => length
0a87520
Erlend E. Aasland added 2 commits April 12, 2022 23:00
Erlend E. Aasland
Don't use magical numbers in exception messages
154edca
Erlend E. Aasland
IT'S ESS CUE EL LITE
f784a81
JelleZijlstra
JelleZijlstra approved these changes Apr 12, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Apr 12, 2022
@JelleZijlstra JelleZijlstra self-assigned this Apr 12, 2022
@JelleZijlstra

JelleZijlstra commented Apr 12, 2022

Copy link
Copy Markdown
Member

Some tests need to be updated with new error messages

All reactions

Sorry, something went wrong.

Uh oh!

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

Erlend E. Aasland
🤦
0bc67ed
@erlend-aasland

erlend-aasland commented Apr 13, 2022

Copy link
Copy Markdown
Contributor Author

FYI, I'll prepare PRs for adding mapping protocol and context manager support.

All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood
AlexWaygood reviewed Apr 14, 2022
View reviewed changes

@AlexWaygood AlexWaygood 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.

Just two small comments; other than that, the docs look good to me! (With the caveat that I know very little about SQLite 😄)

Sorry, something went wrong.

Uh oh!

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

All reactions
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.

Erlend E. Aasland
Address Alex' review
8d834bf
AlexWaygood
AlexWaygood approved these changes Apr 14, 2022
View reviewed changes

@AlexWaygood AlexWaygood 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.

The docs LGTM!

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
@erlend-aasland

erlend-aasland commented Apr 14, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing, both of you :)

AlexWaygood and JelleZijlstra reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

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

Erlend E. Aasland added 2 commits April 15, 2022 00:52
Erlend E. Aasland
Address Berker's comments from pythongh-271: remove unneeded class pr…
2590112
…efix
Erlend E. Aasland
Address Berker's comments from pythongh-271: move __len__ doc to clas…
69a5a1e
…s description
@erlend-aasland

erlend-aasland commented Apr 14, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

BTW, I went over the review comments on the original PR; it seems that the following two comments by Berker are not addressed yet:

  • bpo-24905: Support BLOB incremental I/O in sqlite module #271 (comment)
  • bpo-24905: Support BLOB incremental I/O in sqlite module #271 (comment)

I think we should heed those suggestions. If you are ok with that, I'll fix those right away.

proposed patch
commit 69a5a1e2453710a34bfaf7863f94ea9dacefbedf (HEAD -> sqlite-blob-reduced)
Author: Erlend E. Aasland <[email protected]>
Date:   Fri Apr 15 00:53:09 2022 +0200

    Address Berker's comments from gh-271: move __len__ doc to class description

diff --git a/Doc/library/sqlite3.rst b/Doc/library/sqlite3.rst
index 992256c134..d0274fb797 100644
--- a/Doc/library/sqlite3.rst
+++ b/Doc/library/sqlite3.rst
@@ -1115 +1115,2 @@ Blob Objects
-   data in an SQLite :abbr:`BLOB (Binary Large OBject)`.
+   data in an SQLite :abbr:`BLOB (Binary Large OBject)`.  Call ``len(blob)`` to
+   get the size (number of bytes) of the blob.
@@ -1125,4 +1125,0 @@ Blob Objects
-   .. method:: __len__()
-
-      Return the blob size in bytes.
-

commit 2590112aff8ec625aa5d1debb8fdc10f36fa3017
Author: Erlend E. Aasland <[email protected]>
Date:   Fri Apr 15 00:52:47 2022 +0200

    Address Berker's comments from gh-271: remove unneeded class prefix

diff --git a/Doc/library/sqlite3.rst b/Doc/library/sqlite3.rst
index 0d4dbdf64d..992256c134 100644
--- a/Doc/library/sqlite3.rst
+++ b/Doc/library/sqlite3.rst
@@ -1117 +1117 @@ Blob Objects
-   .. method:: Blob.close()
+   .. method:: close()
@@ -1125 +1125 @@ Blob Objects
-   .. method:: Blob.__len__()
+   .. method:: __len__()
@@ -1129 +1129 @@ Blob Objects
-   .. method:: Blob.read(length=-1, /)
+   .. method:: read(length=-1, /)
@@ -1137 +1137 @@ Blob Objects
-   .. method:: Blob.write(data, /)
+   .. method:: write(data, /)
@@ -1143 +1143 @@ Blob Objects
-   .. method:: Blob.tell()
+   .. method:: tell()
@@ -1147 +1147 @@ Blob Objects
-   .. method:: Blob.seek(offset, origin=os.SEEK_SET, /)
+   .. method:: seek(offset, origin=os.SEEK_SET, /)
AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra

JelleZijlstra commented Apr 14, 2022

Copy link
Copy Markdown
Member

Sounds good, but I think your two patches conflict with each other.

All reactions

Sorry, something went wrong.

Uh oh!

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

JelleZijlstra
JelleZijlstra approved these changes Apr 14, 2022
View reviewed changes
@JelleZijlstra
JelleZijlstra merged commit ee47543 into python:main Apr 15, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 15, 2022
@JelleZijlstra JelleZijlstra mentioned this pull request Apr 15, 2022
bpo-24905: Support BLOB incremental I/O in sqlite module #271
Closed
dholth
dholth reviewed Apr 15, 2022
View reviewed changes
Comment thread Doc/library/sqlite3.rst Resolved

Uh oh!

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

This was referenced Apr 15, 2022
gh-69093: Expose sqlite3.Blob as a class #91550
Merged
sqlite3: add 3.11 additions python/typeshed#7625
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
@erlend-aasland
erlend-aasland deleted the sqlite-blob-reduced branch April 15, 2022 06:08
@palaviv

palaviv commented Apr 15, 2022

Copy link
Copy Markdown
Contributor

@erlend-aasland I wanted to say thank you for making this happen. Crazy it took more then 6 years to merge this...

erlend-aasland reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Apr 15, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

@erlend-aasland I wanted to say thank you for making this happen. Crazy it took more then 6 years to merge this...

Thanks for paving the way, and thanks for having done so much for the sqlite3 module. Yeah, PRs can end up hanging for years before being merged. Better late than never, I guess :)

(Also, I hope you're ok with the changes I've applied to your code.)

All reactions

Sorry, something went wrong.

Uh oh!

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

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
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 30, 2026
sqlite3: Blob crashes when using a negative-step slice #150449
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@AlexWaygood AlexWaygood AlexWaygood approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag
+1 more reviewer
@dholth dholth dholth left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

@JelleZijlstra JelleZijlstra

Labels

extension-modules C modules in the Modules dir

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.

8 participants

@erlend-aasland @bedevere-bot @AlexWaygood @JelleZijlstra @palaviv @dholth @kumaraditya303 @the-knights-who-say-ni
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.