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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnLnhsH25Nnv7LHhekUfPd

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:
 

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

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))

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]