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-100414: Add SQLite backend to dbm by erlend-aasland · Pull Request #114481 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Jan 23, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Initial version authored by Raymond in #100414 (comment).

  • Issue: Add sqlite3 as another possible backing store for the dbm module #100414

📚 Documentation preview 📚: https://cpython-previews--114481.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

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

All reactions
@erlend-aasland
pythongh-100414: Add SQLite backend to dbm
8c19c81
@bedevere-app bedevere-app Bot mentioned this pull request Jan 23, 2024
Add sqlite3 as another possible backing store for the dbm module #100414
Closed
@erlend-aasland
erlend-aasland requested a review from rhettinger January 23, 2024 12:10
@erlend-aasland

erlend-aasland commented Jan 23, 2024

Copy link
Copy Markdown
Contributor Author

cc. @presidento

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Jan 23, 2024

Copy link
Copy Markdown
Contributor Author

I suggest to focus this PR on the basic, required functionality only. We can optimise things in follow-up PRs.

All reactions

Sorry, something went wrong.

Uh oh!

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

erlend-aasland added 2 commits January 23, 2024 13:42
@erlend-aasland
Preliminary docs
7f32772
@erlend-aasland
Decode path a little bit earlier
3f8a119
erlend-aasland
erlend-aasland commented Jan 23, 2024
View reviewed changes
Comment thread Doc/library/dbm.rst Resolved

Uh oh!

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

@erlend-aasland
Merge branch 'main' into sqlite/dbm
4f82c88
erlend-aasland
erlend-aasland commented Jan 23, 2024
View reviewed changes
Comment thread Doc/library/dbm.rst Resolved

Uh oh!

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

@erlend-aasland
Substitutions are already in place
38a9b41
erlend-aasland
erlend-aasland commented Jan 23, 2024
View reviewed changes
Comment thread Doc/library/dbm.rst Resolved

Uh oh!

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

erlend-aasland and others added 7 commits January 23, 2024 20:57
@erlend-aasland
Use CSV-style table, as the rest of the .rst file does
db0f148
@erlend-aasland
dbm.sqlite3.error is a subclass of OSError
e50d1e9
@erlend-aasland
Don't raise DB API exceptions; test this
0abd7dc
@erlend-aasland
Pull in main
98fba0f
@erlend-aasland
Don't panic on double close(); add more tests
2b97160
@erlend-aasland
Corruption tests: check all basic operations for flags 'r', 'w', 'c'
917bfba
@erlend-aasland
Add read-only specific test
874fb81
@erlend-aasland
erlend-aasland marked this pull request as ready for review January 23, 2024 22:51
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner January 23, 2024 22:51
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 23, 2024
@erlend-aasland

erlend-aasland commented Jan 23, 2024

Copy link
Copy Markdown
Contributor Author

I think the basic functionality is ready for an initial round of reviews.

Tests

There's a basic test suite in Lib/test_dbm.py, and I also added some dbm.sqlite3 specific tests in Lib/test_dbm_sqlite3.py. We can expand the latter in follow-up PRs.

Docs

I also added som basic docs; unless significant information is missing, I suggest we work on the docs in follow-up PRs.

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland erlend-aasland mentioned this pull request Jan 23, 2024
dbm.sqlite proof of concept #48033
Closed
gpshead
gpshead reviewed Jan 24, 2024
View reviewed changes
Comment thread Lib/dbm/__init__.py Resolved

Uh oh!

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

erlend-aasland added 5 commits January 24, 2024 09:21
@erlend-aasland
Address review: always import sqlite3; let ImportError deal with miss…
b655473
…ing deps
@erlend-aasland
Test whichdb
95eba93
@erlend-aasland
test namespacing
54751ba
@erlend-aasland
Test that 'c' works even if you already have a database; make sure BU…
906748e
…ILD_TABLE never can raise a DB API exception
@erlend-aasland
Pull in main
7320f18
erlend-aasland
erlend-aasland commented Jan 26, 2024
View reviewed changes
Comment thread Doc/library/dbm.rst Resolved

Uh oh!

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

erlend-aasland and others added 5 commits January 26, 2024 16:15
@erlend-aasland
Update Doc/library/dbm.rst
926ef1a
@erlend-aasland
Add support for 'mode' param in dbm.sqlite3.open()
c0878d9
@erlend-aasland
Pull in main
b7111ee
@erlend-aasland
Pull in main
48972a6
@erlend-aasland
Fix test_misuse_reinit()
d6d7c66
@erlend-aasland
erlend-aasland requested a review from serhiy-storchaka February 1, 2024 22:37
@erlend-aasland

erlend-aasland commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

I made a core dev poll on Discourse regarding if the SQLite backend should be the default one; 16 out of 21 was in favour (Skip changed his mind post voting). This PR does not make the SQLite backend the default. If we want to change the default, we can do it post merging the feature itself. Another possibility, suggested by @ethanfurman:

Shouldn’t we make it the default now so it can be tested in the alphas? We can undo the default portion if it turns out to be a problem.

@serhiy-storchaka, should we land this?

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland
Merge branch 'main' into sqlite/dbm
bc849c3
@serhiy-storchaka

serhiy-storchaka commented Feb 14, 2024

Copy link
Copy Markdown
Member

It seems that currently it is not compatible with other dbm implementations. They all are bytes oriented, but accept strings as input (it is a Python 2 legacy). Most of the dbm tests use string input, so I expect that many user code does it too. But the output always expected to be bytes.

Supporting other types, supported by Sqlite looks attractive, but we implement a backend to dbm, so it should be compatible. We can later add an option to alter the behavior, but it should not be default.

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland
Address Serhiy's offline remark: coerce keys/values to bytes
e782fad
@erlend-aasland

erlend-aasland commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

It seems that currently it is not compatible with other dbm implementations. They all are bytes oriented, but accept strings as input (it is a Python 2 legacy). Most of the dbm tests use string input, so I expect that many user code does it too. But the output always expected to be bytes.

Supporting other types, supported by Sqlite looks attractive, but we implement a backend to dbm, so it should be compatible. We can later add an option to alter the behavior, but it should not be default.

As discussed in DM, I agree with this. I addressed your concern in e782fad.

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland
Align docs to e782fad
b1b9a9b
serhiy-storchaka
serhiy-storchaka approved these changes Feb 14, 2024
View reviewed changes

@serhiy-storchaka serhiy-storchaka 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.

LGTM if all operations still accept strings.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/test/test_dbm_sqlite3.py
# (raw, coerced)
(42, b"42"),
(3.14, b"3.14"),
("string", b"string"),

@serhiy-storchaka serhiy-storchaka Feb 14, 2024

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.

Does it work with non-ASCII strings?

Sorry, something went wrong.

Uh oh!

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

All reactions

@erlend-aasland erlend-aasland Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

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.

Do you mean such a test?

diff --git a/Lib/test/test_dbm_sqlite3.py b/Lib/test/test_dbm_sqlite3.py
index 7bc2a03035..3d2dc5fd55 100644
--- a/Lib/test/test_dbm_sqlite3.py
+++ b/Lib/test/test_dbm_sqlite3.py
@@ -214,6 +214,7 @@ class DataTypes(_SQLiteDbmTests):
         (3.14, b"3.14"),
         ("string", b"string"),
         (b"bytes", b"bytes"),
+        ("æøå", b"\xc3\xa6\xc3\xb8\xc3\xa5"),
     )
 
     def setUp(self):

Sorry, something went wrong.

Uh oh!

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

All reactions
@erlend-aasland
Compat with other backends: silently coerce keys to bytes
34930cb
@erlend-aasland

erlend-aasland commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

With 34930cb, the dbm.sqlite3 backend now aligns with the other backends; for example, the general code snippet in the dbm docs now works as expected with the new backend.

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Feb 14, 2024

Copy link
Copy Markdown
Contributor Author

Let's land this, @serhiy-storchaka. There is plenty of opportunities to tweak the implementation and to expand the test suite.

Thanks @smontanaro for the initial idea, @rhettinger for the initial implementation, and everyone else involved for ideas, reviews and critique. Thanks to @serhiy-storchaka for the, as always, extremely thorough review of the feature and the implementation.

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland
erlend-aasland enabled auto-merge (squash) February 14, 2024 11:02
@erlend-aasland
erlend-aasland merged commit dd5e4d9 into python:main Feb 14, 2024
@erlend-aasland
erlend-aasland deleted the sqlite/dbm branch February 14, 2024 11:14
@bedevere-app bedevere-app Bot removed the awaiting merge label Feb 14, 2024
@bedevere-bot

This comment was marked as resolved.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@erlend-aasland

erlend-aasland commented Feb 14, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

[...]

Failed tests:

  • test_dbm_sqlite3

[...]
ModuleNotFoundError: No module named '_sqlite3'

Proposed fixes:

  • gh-100414: Skip test_dbm_sqlite3 if sqlite3 is unavailable #115448
  • gh-100414: Skip test_dbm_sqlite3 if sqlite3 is unavailable #115449
All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot

This comment was marked as duplicate.

Sign in to view
erlend-aasland added a commit to erlend-aasland/cpython that referenced this pull request Feb 14, 2024
@erlend-aasland
Amend pythongh-114481 NEWS and What's New entries
fb9dd21
@bedevere-bot

This comment was marked as resolved.

Sign in to view
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this pull request Feb 14, 2024
@erlend-aasland @rhettinger
@serhiy-storchaka @felixxm @fsc-eriker
pythongh-100414: Add SQLite backend to dbm (python#114481)
eb753c1
Co-authored-by: Raymond Hettinger <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
Co-authored-by: Mariusz Felisiak <[email protected]>
@meghprkh meghprkh mentioned this pull request Mar 30, 2025
dbm.sqlite breaks multi-threaded shelve usage #131918
Open
@guillaume-fr guillaume-fr mentioned this pull request Jul 23, 2025
Python 3.13 compatibility issue: SQLite objects created in a thread can only be used in that same thread. ynohat/bossman#46
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead left review comments
@corona10 corona10 corona10 approved these changes
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@rhettinger rhettinger Awaiting requested review from rhettinger
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag
+2 more reviewers
@presidento presidento presidento left review comments
@felixxm felixxm felixxm left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

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

@erlend-aasland @corona10 @serhiy-storchaka @bedevere-bot @gpshead @presidento @felixxm
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.