Skip to content

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Aug 7, 2025

erlend-aasland and others added 2 commits August 7, 2025 10:08
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@erlend-aasland erlend-aasland marked this pull request as ready for review August 7, 2025 09:01
@erlend-aasland erlend-aasland moved this from Todo to In Progress in Docs PRs Aug 7, 2025
Copy link
Member

@StanFromIreland StanFromIreland left a 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.

@erlend-aasland
Copy link
Contributor Author

Looks fine to me.

Thanks for the review.

PS. You may have accidentally added the 3.12 back port label.

autocommit was added in 3.12; I prefer to also amend the 3.12 docs.

@zzzeek: would you like to review this?

@erlend-aasland
Copy link
Contributor Author

I made some amendments:

@erlend-aasland
Copy link
Contributor Author

cc. @layday


.. testcode::

cur = con.cursor()
Copy link

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.

Copy link
Contributor Author

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.

Copy link

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:

  1. a context manager
with conn.autocommit_block:
    # ...
  1. a PRAGMA method
   cursor.pragma("FOREIGN KEYS=ON")
  1. 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".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. 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.
  2. & 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.

Copy link

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 with autocommit=True before flipping it off.
  • It has the appearance of a bodge, though it's decidedly less bad than manually executing a ROLLBACK or COMMIT followed by a BEGIN.
  • 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:

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review docs Documentation in the Doc dir needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes skip news
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants