From ffec11cfb7875120e34d52d89dc5a06e79de0f9d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 18:46:09 +0000 Subject: [PATCH] enable_wal() and disable_wal() refuse to run inside a transaction Changing the journal mode assigns conn.isolation_level, which commits any open transaction as a side effect - silently breaking the rollback guarantee of atomic() blocks and of user-managed transactions. Both methods now raise RuntimeError if a transaction is open. Calling them when the database is already in the requested mode remains a no-op. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UnLnhsH25Nnv7LHhekUfPd --- docs/changelog.rst | 1 + docs/python-api.rst | 2 ++ sqlite_utils/db.py | 21 ++++++++++++++++++++- tests/test_wal.py | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 3a36722..0c409a1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,7 @@ Unreleased - ``table.upsert()`` and ``table.upsert_all()`` now detect the primary key or compound primary key of an existing table, so the ``pk=`` argument is no longer required when upserting into a table that already has a primary key. - ``table.upsert()`` and ``table.upsert_all()`` now raise ``PrimaryKeyRequired`` if a record is missing a value for any primary key column, or has a value of ``None`` for one. Previously such records - which can never match an existing row - were quietly inserted as brand new rows, or triggered a confusing ``KeyError`` after the insert had already taken place. - ``db.table(table_name).insert({})`` can now be used to insert a row consisting entirely of default values into an existing table, using ``INSERT INTO ... DEFAULT VALUES``. (:issue:`759`) +- ``db.enable_wal()`` and ``db.disable_wal()`` now raise a ``RuntimeError`` if called while a transaction is open. Previously they would silently commit the open transaction as a side effect of changing the journal mode, breaking the rollback guarantee of ``db.atomic()`` and of user-managed transactions. .. _v4_0rc1: diff --git a/docs/python-api.rst b/docs/python-api.rst index ae2c0cb..f03f963 100644 --- a/docs/python-api.rst +++ b/docs/python-api.rst @@ -2702,6 +2702,8 @@ You can disable WAL mode using ``.disable_wal()``: Database("my_database.db").disable_wal() +The journal mode can only be changed outside of a transaction. Calling either method while a transaction is open - inside a ``db.atomic()`` block, for example - raises a ``RuntimeError``, unless the database is already in the requested mode in which case the call is a no-op. + You can check the current journal mode for a database using the ``journal_mode`` property: .. code-block:: python diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index b4f5dcc..97e8920 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -854,17 +854,36 @@ class Database: def enable_wal(self) -> None: """ Sets ``journal_mode`` to ``'wal'`` to enable Write-Ahead Log mode. + + :raises RuntimeError: if called while a transaction is open - the + journal mode can only be changed outside of a transaction """ if self.journal_mode != "wal": + self._ensure_no_open_transaction("enable_wal()") with self.ensure_autocommit_off(): self.execute("PRAGMA journal_mode=wal;") def disable_wal(self) -> None: - "Sets ``journal_mode`` back to ``'delete'`` to disable Write-Ahead Log mode." + """ + Sets ``journal_mode`` back to ``'delete'`` to disable Write-Ahead Log mode. + + :raises RuntimeError: if called while a transaction is open - the + journal mode can only be changed outside of a transaction + """ if self.journal_mode != "delete": + self._ensure_no_open_transaction("disable_wal()") with self.ensure_autocommit_off(): self.execute("PRAGMA journal_mode=delete;") + def _ensure_no_open_transaction(self, operation: str) -> None: + # Changing journal mode assigns conn.isolation_level, which commits + # any open transaction as a side effect - breaking the rollback + # guarantee of atomic() and of user-managed transactions + if self.conn.in_transaction: + raise RuntimeError( + "{} cannot be used while a transaction is open".format(operation) + ) + def _ensure_counts_table(self) -> None: with self.atomic(): self.execute(_COUNTS_TABLE_CREATE_SQL.format(self._counts_table_name)) diff --git a/tests/test_wal.py b/tests/test_wal.py index 23ca144..fa3c855 100644 --- a/tests/test_wal.py +++ b/tests/test_wal.py @@ -21,3 +21,39 @@ def test_enable_disable_wal(db_path_tmpdir): db.disable_wal() assert "delete" == db.journal_mode assert "test.db-wal" not in [f.basename for f in tmpdir.listdir()] + + +def test_enable_wal_inside_transaction_raises(db_path_tmpdir): + db, path, tmpdir = db_path_tmpdir + db["test"].insert({"id": 1}, pk="id") + with pytest.raises(RuntimeError): + with db.atomic(): + db["test"].insert({"id": 2}, pk="id") + db.enable_wal() + # The atomic() block must have rolled back cleanly and the + # journal mode must be unchanged + assert db.journal_mode == "delete" + assert [r["id"] for r in db["test"].rows] == [1] + + +def test_disable_wal_inside_transaction_raises(db_path_tmpdir): + db, path, tmpdir = db_path_tmpdir + db.enable_wal() + db["test"].insert({"id": 1}, pk="id") + with pytest.raises(RuntimeError): + with db.atomic(): + db["test"].insert({"id": 2}, pk="id") + db.disable_wal() + assert db.journal_mode == "wal" + assert [r["id"] for r in db["test"].rows] == [1] + + +def test_enable_wal_noop_inside_transaction_is_allowed(db_path_tmpdir): + # Calling enable_wal() when WAL is already enabled is a no-op, + # so it is fine inside a transaction + db, path, tmpdir = db_path_tmpdir + db.enable_wal() + with db.atomic(): + db["test"].insert({"id": 1}, pk="id") + db.enable_wal() + assert [r["id"] for r in db["test"].rows] == [1]