mirror of
https://github.com/simonw/datasette.git
synced 2026-09-10 10:34:06 +02:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N76afGMhBRQk528VF1LTpR
This commit is contained in:
parent
84fb4221ab
commit
1ff4e67b79
4 changed files with 83 additions and 4 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
||||
|
|
|
|||
|
|
@ -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() <https://docs.python.org/3/library/sqlite3.html#sqlite3.Cursor.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() <https://docs.python.org/3/library/sqlite3.html#sqlite3.Cursor.executescript>`__ method instead, where each statement is committed as it executes.
|
||||
|
||||
.. _database_execute_write_many:
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue