-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
gh-137205: Document how to safely use PRAGMA during SQLite transactions #137505
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.
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
base: main
Are you sure you want to change the base?
gh-137205: Document how to safely use PRAGMA during SQLite transactions #137505
Conversation
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
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 fine to me.
PS. You may have accidentally added the 3.12 back port label.
Thanks for the review.
@zzzeek: would you like to review this? |
I made some amendments:
|
cc. @layday |
|
||
.. testcode:: | ||
|
||
cur = con.cursor() |
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.
I wish the state of affairs was better here, but while I can think of various API features that would make this less klunky, I dont think any of them would be worth it, so here we are.
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.
I don't think a decorator is worth it. Since PRAGMAs are mostly used during the setup of a database connection, I think @layday's approach is the most sane: connect using autocommit=False
, execute your PRAGMAs, then set the autocommit attr to whatever suits your application best.
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.
my API features would be either:
- a context manager
with conn.autocommit_block:
# ...
- a PRAGMA method
cursor.pragma("FOREIGN KEYS=ON")
- include commonly needed pragmas as part of connect() . A bitwise enum is sort of nice here but doesnt work for pragmas that have numeric arguments
# doesnt work for every kind of pragma but looks nice
conn = sqlite3.connect(...., pragmas=Pragma.FOREIGN_KEYS_ON | Pragma.JOURNAL_MODE_PERSIST)
in your comment I assume you meant "connect with autocommit=True".
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.
- It's a nice idea, but we already have a context manager for transactions. I'm not sure it is worth it to introduce a context manager for the opposite.
- & 3: I would prefer to not add more APIs with side effects. Moreover, I would prefer to not manage an enum of supported PRAGMAs.
[...] in your comment I assume you meant "connect with autocommit=True".
Yes, thanks.
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.
There's a few things I don't like about the proposed workaround:
- It's rather verbose. Pragmas are typically only set once on connect, so it's sufficient to initialise
connect
withautocommit=True
before flipping it off. - It has the appearance of a bodge, though it's decidedly less bad than manually executing a
ROLLBACK
orCOMMIT
followed by aBEGIN
. - Entering autocommit implicitly emits a
COMMIT
, which has a 'spooky action at a distance' feel to it. - Enabling foreign keys is more important than any Python-specific recommendation made by the docs, and if
autocommit=False
doesn't support the FK pragma out of the box, that vastly reduces its usefulness.
I don't really know what'd work better, but I'm gravitating towards an init hook that'd be executed before the first BEGIN
, mirroring SQLA's listens_for
idiom. I don't think that's in the scope of this PR though, so I'd personally be happy if the suggestion was rephrased to:
cur = con.cursor() | |
* Else, initialize the connection with ``autocommit=True`` | |
and set the :attr:`!autocommit` attribute to ``False`` | |
after executing the ``PRAGMA`` statement: | |
.. testcode:: | |
con = sqlite3.connect(":memory:", autocommit=True) | |
cur.execute("PRAGMA foreign_keys = ON") | |
con.autocommit = False # Enable implicit transaction control. |
a context manager
I don't think an autocommit context manager is generally useful, and I can see it being misused to break out of a transaction for any number of reasons, e.g. to fetch changes made by another connection with WAL on.
a PRAGMA method
I don't know how this would work, but SQLite has pragmas which have no effect outside of a transaction - defer_foreign_keys
is one of them. If the idea is that pragma(...)
would exit a transaction to set the provided pragma, then the driver's gonna have to encode specific knowledge of SQLite pragmas.
cc. @zzzeek
📚 Documentation preview 📚: https://cpython-previews--137505.org.readthedocs.build/