From a0387791e511a17eb96e0ed667da222550a00412 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 6 Jul 2026 21:51:31 -0700 Subject: [PATCH] ensure_autocommit_on() raises TransactionError inside a transaction Assigning conn.isolation_level commits any pending transaction as a side effect, so entering the context manager with a transaction open silently committed the caller's work and made a later rollback() a no-op. All internal callers already ensure no transaction is open; the public API now enforces it, matching enable_wal() and disable_wal(). Refs https://github.com/simonw/sqlite-utils/issues/769#issuecomment-4900034150 Co-Authored-By: Claude Fable 5 --- docs/changelog.rst | 1 + sqlite_utils/db.py | 9 +++++++++ tests/test_wal.py | 17 +++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4339e81..87a1ab7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,6 +19,7 @@ Unreleased - Fixed a bug where a failed write statement executed with ``db.execute()`` left the driver's implicit transaction open. Every subsequent write then joined that phantom transaction, which nothing committed, so their work was silently rolled back when the connection was closed. The implicit transaction opened by a failed statement is now rolled back before the exception is raised. A failed write inside a transaction opened with ``db.begin()`` or ``db.atomic()`` leaves that transaction open and untouched, as before. - Fixed a bug where transaction-control statements prefixed with an empty statement - ``db.query("; COMMIT")`` - or a UTF-8 byte order mark slipped past the check that rejects them, committing the caller's open transaction before raising a confusing ``OperationalError``. The keyword scanner used by ``db.query()`` and ``db.execute()`` now skips leading ``;`` and byte order marks, matching what the ``sqlite3`` driver tolerates before the first token, so these statements are rejected with a ``ValueError`` without being executed. The same fix means ``db.execute("; BEGIN")`` no longer auto-commits the transaction it just opened. - Documented a limitation of ``db.query()``: a ``PRAGMA`` statement that returns no rows raises a ``ValueError`` but still takes effect, because PRAGMA statements run outside the savepoint guard used to roll back other rejected statements. Use ``db.execute()`` for row-less PRAGMA statements. +- ``db.ensure_autocommit_on()`` now raises ``TransactionError`` if called while a transaction is open. Assigning ``isolation_level`` commits any pending transaction as a side effect, so entering the block silently committed the caller's open transaction and made a later ``rollback()`` a no-op. - ``sqlite-utils migrate --stop-before`` now exits with an error if the named migration has already been applied. Previously the name passed validation but was only checked against pending migrations, so every migration after it was silently applied - the exact outcome ``--stop-before`` exists to prevent. ``Migrations.apply(db, stop_before=...)`` raises ``ValueError`` in the same situation, before applying anything. - Fixed a regression where ``table.insert(..., pk=..., alter=True)`` raised ``InvalidColumns`` if the primary key column did not exist in the table yet. With ``alter=True`` the check now waits until the record keys are known, so a pk column supplied by the records is added by the alter as it was in 3.x. A pk column found in neither the table nor the records still raises ``InvalidColumns``. - Fixed a bug where inserting CSV or TSV data into an existing table rewrote that table's column types to match the incoming file. Type detection is the default in 4.0, so ``sqlite-utils insert data.db places places.csv --csv`` against a table with a ``TEXT`` zip code column would convert the column to ``INTEGER`` and corrupt values with leading zeros - ``"01234"`` became ``1234``. Detected types are now only applied when the ``insert`` or ``upsert`` command creates the table. diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index faf588f..90e7a83 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -665,7 +665,16 @@ class Database: # do stuff here The previous ``isolation_level`` is restored at the end of the block. + + :raises TransactionError: if a transaction is open - assigning + ``isolation_level`` would commit it as a side effect, silently + breaking the caller's ability to roll back """ + if self.conn.in_transaction: + raise TransactionError( + "ensure_autocommit_on() cannot be used inside a transaction - " + "changing isolation_level would commit the open transaction" + ) old_isolation_level = self.conn.isolation_level try: self.conn.isolation_level = None diff --git a/tests/test_wal.py b/tests/test_wal.py index c5a9c60..2ddcf54 100644 --- a/tests/test_wal.py +++ b/tests/test_wal.py @@ -69,3 +69,20 @@ def test_enable_wal_noop_inside_transaction_is_allowed(db_path_tmpdir): db["test"].insert({"id": 1}, pk="id") db.enable_wal() assert [r["id"] for r in db["test"].rows] == [1] + + +def test_ensure_autocommit_on_inside_transaction_raises(db_path_tmpdir): + # Setting isolation_level commits any pending transaction as a side + # effect, silently breaking the caller's rollback guarantee - so + # entering autocommit mode with a transaction open is an error + db, path, tmpdir = db_path_tmpdir + db["test"].insert({"id": 1}, pk="id") + db.begin() + db.execute("insert into test (id) values (2)") + with pytest.raises(TransactionError): + with db.ensure_autocommit_on(): + pass + # The transaction is still open and can still be rolled back + assert db.conn.in_transaction + db.rollback() + assert [r["id"] for r in db["test"].rows] == [1]