From 1ff4e67b790c3d194a65b8cad09fd1fce7967765 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 05:54:02 +0000 Subject: [PATCH] Make execute_write_script transactional, matching its documentation execute_write_script() was documented as running inside a transaction but actually passed transaction=False and used conn.executescript(), which commits each statement as it executes - a failing script could half-apply. Scripts are now split into complete statements (via sqlite3.complete_statement) and executed one at a time inside the task transaction, so a failing script applies nothing. Scripts containing statements that cannot run in a transaction (VACUUM, ATTACH, DETACH, PRAGMA) or that manage transactions themselves (BEGIN, COMMIT, SAVEPOINT etc) keep the previous executescript() autocommit behavior. Refs #2831 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01N76afGMhBRQk528VF1LTpR --- datasette/database.py | 50 ++++++++++++++++++++++++++++++-- docs/changelog.rst | 1 + docs/internals.rst | 6 ++-- tests/test_internals_database.py | 30 +++++++++++++++++++ 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/datasette/database.py b/datasette/database.py index e8271c63..82522e6a 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -59,6 +59,44 @@ def _can_execute_in_transaction(sql): return match.group(1).lower() not in _STATEMENTS_DISALLOWED_IN_TRANSACTION +# Scripts that manage their own transactions also cannot run inside the +# task transaction +_SCRIPT_STATEMENTS_DISALLOWED_IN_TRANSACTION = _STATEMENTS_DISALLOWED_IN_TRANSACTION | { + "begin", + "commit", + "end", + "rollback", + "savepoint", + "release", +} + + +def _script_can_execute_in_transaction(statements): + for statement in statements: + match = _FIRST_KEYWORD_RE.match(statement) + if ( + match is not None + and match.group(1).lower() in _SCRIPT_STATEMENTS_DISALLOWED_IN_TRANSACTION + ): + return False + return True + + +def _iter_sql_statements(sql): + # Split a multi-statement SQL string into complete statements using + # sqlite3.complete_statement() + statement = [] + for char in sql: + statement.append(char) + statement_sql = "".join(statement).strip() + if statement_sql and sqlite3.complete_statement(statement_sql): + yield statement_sql + statement = [] + remainder = "".join(statement).strip() + if remainder: + yield remainder + + def _run_write_in_transaction(conn, fn): # Open the transaction explicitly instead of relying on the sqlite3 # driver's implicit BEGIN, which only fires on the first data-modifying @@ -302,13 +340,21 @@ class Database: async def execute_write_script(self, sql, block=True, request=None): self._check_not_closed() + statements = list(_iter_sql_statements(sql)) + transaction = _script_can_execute_in_transaction(statements) def _inner(conn): - return conn.executescript(sql) + if transaction: + # Execute statements one at a time so they run inside the + # task transaction - conn.executescript() would commit it + for statement in statements: + conn.execute(statement) + else: + return conn.executescript(sql) with trace("sql", database=self.name, sql=sql.strip(), executescript=True): results = await self.execute_write_fn( - _inner, block=block, transaction=False, request=request + _inner, block=block, transaction=transaction, request=request ) return results diff --git a/docs/changelog.rst b/docs/changelog.rst index f58b1c75..1f124fcc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,6 +11,7 @@ Unreleased - Write functions run via ``await db.execute_write_fn()`` now execute inside an explicitly opened ``BEGIN IMMEDIATE`` transaction, committed when the function returns or rolled back if it raises. Previously the transaction was only opened implicitly by the first raw data-modifying statement, which meant writes made through sqlite-utils committed independently mid-task - a function that used sqlite-utils and then failed could leave those writes permanently committed. sqlite-utils write methods now nest inside the task transaction as savepoints, so a failing write function rolls back everything it did. Functions run with ``transaction=True`` should no longer manage transactions themselves - use ``transaction=False`` for manual transaction control. (:issue:`2831`) - ``await db.execute_write()`` detects statements that SQLite cannot execute inside a transaction - ``VACUUM``, ``ATTACH``, ``DETACH`` and ``PRAGMA`` - and runs them in autocommit mode instead. (:issue:`2831`) +- ``await db.execute_write_script()`` is now transactional, matching its documentation: if any statement in the script fails, none of its statements are applied. Scripts containing statements that cannot run inside a transaction, or that manage transactions themselves, fall back to the previous ``conn.executescript()`` autocommit behavior. (:issue:`2831`) .. _v1_0_a36: diff --git a/docs/internals.rst b/docs/internals.rst index 98fdcd82..9a5f3f50 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -2066,9 +2066,11 @@ Each call to ``execute_write()`` will be executed inside a transaction, with the await db.execute_write_script(sql, block=True) ---------------------------------------------- -Like ``execute_write()`` but can be used to send multiple SQL statements in a single string separated by semicolons, using the ``sqlite3`` `conn.executescript() `__ method. +Like ``execute_write()`` but can be used to send multiple SQL statements in a single string separated by semicolons. -Each call to ``execute_write_script()`` will be executed inside a transaction. +Each call to ``execute_write_script()`` will be executed inside a transaction - if any statement fails, none of the statements will be applied. + +The exception is scripts that include statements which SQLite cannot execute inside a transaction - ``VACUUM``, ``ATTACH``, ``DETACH``, ``PRAGMA`` - or that manage transactions themselves using ``BEGIN``, ``COMMIT``, ``ROLLBACK``, ``SAVEPOINT`` or ``RELEASE``. Those scripts are executed using the ``sqlite3`` `conn.executescript() `__ method instead, where each statement is committed as it executes. .. _database_execute_write_many: diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index 1ca5ee77..74818df9 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -179,6 +179,36 @@ async def test_execute_write_fn_sqlite_utils_integrity_error_rolls_back_task(): assert count == 0 +@pytest.mark.asyncio +async def test_execute_write_script_is_transactional(): + # https://github.com/simonw/datasette/issues/2831 + # A script that fails part way through should apply none of its statements + datasette = Datasette(memory=True) + db = datasette.add_memory_database("test_write_script_txn") + with pytest.raises(sqlite3.OperationalError): + await db.execute_write_script( + "create table one (id integer primary key);\n" + "insert into one (id) values (1);\n" + "insert into no_such_table (id) values (2);" + ) + assert "one" not in await db.table_names() + + +@pytest.mark.asyncio +async def test_execute_write_script_with_transaction_unsafe_statements(tmp_path): + # Scripts containing statements that cannot run inside a transaction + # (VACUUM, BEGIN etc) still execute, using the previous autocommit behavior + path = str(tmp_path / "test.db") + sqlite3.connect(path).close() + datasette = Datasette([path]) + db = datasette.get_database("test") + await db.execute_write_script("create table t (id integer primary key);\nvacuum;") + assert "t" in await db.table_names() + await db.execute_write_script("begin;\ninsert into t (id) values (1);\ncommit;") + count = (await db.execute("select count(*) from t")).single_value() + assert count == 1 + + @pytest.mark.asyncio async def test_execute_write_fn_writes_invisible_to_readers_until_task_ends(tmp_path): # https://github.com/simonw/datasette/issues/2831