Repository navigation
The sqlite3 context manager does not work with isolation_level=None #61162
Description
Activity
Its operation is also not particularly intuitive if isolation_level is not None, so its documentation needs some clarification.
Currently the transaction manager does nothing on enter, and does a commit or rollback on exit, depending on whether or not there was an exception inside the with block. With isolation_level set to None, the sqlite3 library is in autocommit mode, so changes will get committed immediately inside the with, which is simply broken.
If isolation_level is not None, then the behavior of the transaction manager depends heavily on what happens inside the with block. If the with block contains only the defined DQL statements (insert, update, delete, replace) and select statements, then things will work as expected. However, if another statement (such as a CREATE TABLE or a PRAGMA) is included in the with block, an intermediate commit will be done and a new transaction started.
I propose to do two things to fix this issue: explain the above in the transactions manager docs, and have the context manager check to see if we are in isolation_level None, and if so, issue a begin (and then document that as well).
One question is, can the fix be backported? It will change the behavior of code that doesn't throw an error, but most such code won't be doing what the author expected (run the with block inside a transaction...in pure autocommit mode the transaction manager is a no-op). One place code could break is if someone figured out this issue and worked around it by explicitly starting a transaction before (or after) entering the with block. In this case they would now get an error that a transaction cannot be started inside another. I would think this is unlikely...the more obvious workaround would be to write a custom transaction manager, so I suspect that that is what is actually in the field. But that's a (hopeful :) guess.
A fix for this problem would be to use 'savepoint' instead of 'begin' if the sqlite3 version supports it (it is apparently supported as of 3.6.8).
So, I'd like to see the fix, conditionally using SAVEPOINT, (once it written and tested) applied to all active python versions, but am open to the argument that it shouldn't be.
loewis commented
on Jan 14, 2013 loewismannequinMannequin"changes will get committed immediately inside the with, which is simply broken"
What do you mean by that?
A. Changes ought to be committed immediately, but are not; it is broken, and changes must be committed immediately.- or -
B. What actually happens is that changes are committed immediately, and sqlite is incorrect in doing so.
Your discussion suggests B; in this case, I disagree that there is a bug. In auto-commit mode, it should really auto-commit, regardless of context managers. The context manager documentation doesn't claim otherwise.
- or -
B, yes.
So you would view the connection context manager acting as an actual transaction manager as a new feature? Would you be OK with adding that feature to the existing context manager in 3.4 (since currently the context manager is a noop in autocommit mode), or do you think we need to create a new context manager for this? Or do we do as the issue that sparked this (bpo-8145) suggested, and just document how to create your own?
aymericaugustin commented
on Jun 6, 2014 aymericaugustinmannequinMannequin- Thesis *
I belive that using the connection as a context manager is an inadequate API for controlling transactions because it's very likely to result in subtly broken code.
As a consequence, my recommendation would be to deprecate this API.
- Argumentation *
If you nest a naive transaction context manager (BEGIN / COMMIT / ROLLBACK), you'll get very lousy transaction semantics. Look at this example:
with connection: # outer transaction with connection: # inner transaction do_X_in_db() do_Y_in_db()
# once in a while, you get an exception there...With this code, when you get an exception, X will be presevred in the database, but not Y. Most likely this breaks the expectations of the "outer transaction". Now, imagine the inner transaction in deep inside a third-party library, and you understand that this API is a Gun Pointed At Feet.
Of course, you could say "don't nest", but:
- this clashes with the expected semantics of Python context managers,
- it's unreasonable to expect Python users to audit all their lower level libraries for this issue!
Now, let's look at how popular database-oriented libraires handle this.
SQLAlchemy provides an explicit begin() method: http://docs.sqlalchemy.org/en/latest/core/connections.html#sqlalchemy.engine.Connection.begin
It also provides variants for nested transactions and two-phase commits.
Django provide an all-purpose atomic() context manager:
https://docs.djangoproject.com/en/stable/topics/db/transactions/#django.db.transaction.atomicThat function takes a keyword argument,
savepoint, to control whether a savepoint is emitted for nested transactions.So it's possible to implement a safe, nestable context manager with savepoints. However:
- you need to provide more control, and as a consequence you cannot simply use the connection as a context manager anymore;
- it takes a lot of rather complex code. See Django's implementation for an example:
https://github.com/django/django/blob/stable/1.6.x/django/db/transaction.py#L199-L372
If you ignore the cross-database compatibility stuff, you're probably still looking at over a hundred lines of very stateful code...
That's why I believe it's better to leave this up to user code, and to stop providing an API that looks good for trivial use cases but that's likely to introduce subtle transactional integrity bugs.
ghaering commented
on Aug 19, 2015 ghaeringmannequinMannequinI'm +1 on deprecating the connection manager
This issue is no newcomer friendly, I remove the "easy" keyword.
coleifer commented
on Aug 12, 2019 coleifermannequinMannequinWith isolation_level set to None, the sqlite3 library is in autocommit mode, so changes will get committed immediately inside the with, which is simply broken.
Not necessarily. When sqlite is in autocommit mode, you can still open transactions by executing a BEGIN query. In fact, that's the main reason to use isolation_level=None -- you can manage the transactions yourself.
Kodiologist commented
on Mar 25, 2022 KodiologistmannequinMannequinThis bit me real bad. On Python 3.8, I wrote a program with
isolation_level = Noneandwith db: …and spent a long time figuring out why writes were so slow. Turns out thatwith dbdoesn't actually start a transaction in this case, as the documentation suggests it should. This issue is approaching the age of 10, so if there's still uncertainty about how the implementation or the interface should change, the docs should be clarified in the meantime.I always thought the Python library turning off autocommit by default, contrary to SQLite's command-line interface, was needlessly surprising. I think it contributed to this problem because the docs about context managers seem to assume you have autocommit off.
Reacted by phiresky7 remaining items
- added 3 commits that reference this issue
on Jun 19, 2022 IMO, we can close this issue now.
I think the discussion of deprecating the current context manager and/or possibly replacing it with a new SAVEPOINT-based context manager should have a wider audience than this issue alone. I therefore suggest opening a topic on Discourse, to try and gain more attention to such changes. If there is sufficient interest in following up and implementing the deprecation and/or a new context manager, I suggest we create new issue on the bug tracker, with backlinks to this discussion for reference.
- Repository owner moved this from Backwards compatibility issues to Done in sqlite3 issues
on Jun 19, 2022 - added a commit that references this issue
on Jun 26, 2022 I just hit the same issue as @Kodiologist
Specifically what happens:
First, you use sqlite3 library like this:
db = sqlite3.connect("x.db") db.execute("create table foo(x);") # this auto-commits, as expected. `$ sqlite3 x.db .dump` shows the table as expected. db.execute("insert into foo values (1);") # this implicitly creates a cursor and a transaction but doesn't commit it!! db.close() # $ sqlite3 x.db .dump # the table is empty! why?
After a while of thinking your code is buggy you find out that sqlite3 uses isolation_level=DEFERRED by default which starts a transaction "secretly" and is different than the default of the sqlite3 REPL (and pretty much every other database engine I've seen)
So you set isolation_level=None to get the behaviour you expected. But then some time later you write this code:
db = sqlite3.connect("x.db", isolation_level=None) db.execute("insert into foo values (1);") # this one autocommits as expected with db: for i in range(1000): db.execute("insert into foo values (1);") # i would expect these all to happen in one transaction, but turns out it commits after every insert # (you probably won't even realize this until later just thinking sqlite itself is slow) db.execute("syntax error") # the table contains the value that you think was rolled back # but it turns out `with db` doesn't actually do anything when isolation_mode=None
I think this behaviour is really confusing for most people, at least those coming from SQLite in different programming langugaes as well as other db engines like PostgreSQL.
Here's my workaround:
class ConnectionFixedCtxManager(sqlite3.Connection): def __enter__(self) -> ConnectionFixedCtxManager: raise Exception( "don't use contextmanager on sqlite3 connection, it's broken with isolation_level=None (which is the sane default). use .transaction() instead (see https://github.com/python/cpython/issues/61162)" ) def transaction(self) -> CursorFixedCtxManager: return CursorFixedCtxManager(self) class CursorFixedCtxManager(ContextManager[sqlite3.Cursor]): _cursor: sqlite3.Cursor | None = None def __init__(self, conn: ConnectionFixedCtxManager): self._conn = conn def __enter__(self) -> sqlite3.Cursor: self._cursor = self._conn.cursor() self._cursor.execute("BEGIN") return self._cursor def __exit__( self, exc_type: type[BaseException] | None, value: BaseException | None, traceback: TracebackType | None, ) -> None: assert self._cursor if exc_type is None: self._conn.commit() else: self._conn.rollback() self._cursor.close() self._cursor = None
Usage:
db = sqlite3.connect(..., isolation_level=None, factory=ConnectionFixedCtxManager) with db.transaction() as cursor: cursor.execute("insert into foo values (1);")
Pretty simiilar to the .savepoint() suggestion by @erlend-aasland above except I think the context manager should return a cursor to prevent mixing up the main db object with the cursor. My code doesn't do anything for nested transactions since I don't use those. The separation between connection and cursor is still not really clean, IMO all convenience methods like
db.execute()should throw an error when .transaction() was called /in_transaction == TrueEdit: I see there's some efforts to maybe fix this soon. Awesome!
Reacted by Donal FellowsThe separation between connection and cursor is still not really clean, IMO all convenience methods like db.execute() should throw an error when .transaction() was called / in_transaction == True
That would probably break existing code, so that is not an option.
The docs have been made more explicit in the last weeks. Hopefully that will reduce the confusion. Also, the upcoming autocommit attribute will introduce a cleaner API that should be easier to understand, imo.
It wouldn't be a breaking change if it only happens with a new
.savepoint()/.transaction()method right? My idea would be that while the context manager created from that function is open, the mainConnectionobject would basically be disabled and not usable.Otherwise the
__enter__method should probably returnNoneand notConnection(as the current contextmanager does) orCursor(as my suggestion). If it returns something I'd expect what it returns to be different and statements on the main connection object to be unaffectedLet's move this discussion to either Discourse or a new issue.
Reacted by phiresky
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: