mirror of
https://github.com/simonw/sqlite-utils.git
synced 2026-07-24 18:04:32 +02:00
Transform now refuses to run inside a transaction if destructive foreign keys exist (#795)
* Transform now refuses to run inside a transaction if destructive foreign keys exist Closes #794 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014StVTWQJpFhfZJK2CYVBwv
This commit is contained in:
Simon Willison
GitHub
parent
d714200659
commit
f66ddcb215
3 changed files with 190 additions and 2 deletions
|
|
@ -434,9 +434,10 @@ The library will never commit a transaction you opened. If you call write method
|
|||
|
||||
Prefer ``db.atomic()`` or ``db.begin()``, ``db.commit()`` and ``db.rollback()`` over mixing sqlite-utils transaction methods with calls to ``db.conn.commit()``, ``db.conn.rollback()`` or raw transaction-control SQL. Mixing the two layers makes it much harder to tell which layer owns the current transaction.
|
||||
|
||||
Two related safeguards to be aware of:
|
||||
Some related safeguards to be aware of:
|
||||
|
||||
- ``db.enable_wal()`` and ``db.disable_wal()`` raise a ``sqlite_utils.db.TransactionError`` if called while a transaction is open, because changing the journal mode would commit it as a side effect.
|
||||
- ``table.transform()`` raises a ``sqlite_utils.db.TransactionError`` if called while a transaction is open with ``PRAGMA foreign_keys`` enabled and the table is referenced by foreign keys with destructive ``ON DELETE`` actions, because the pragma cannot be turned off mid-transaction to protect those referencing rows - see :ref:`python_api_transform_foreign_keys_transactions`.
|
||||
- Closing the database - explicitly with ``db.close()``, or by exiting a ``with Database(...) as db:`` block - rolls back any transaction that is still open, see :ref:`python_api_close`.
|
||||
|
||||
.. _python_api_transactions_modes:
|
||||
|
|
@ -1996,6 +1997,36 @@ If you want to do something more advanced, you can call the ``table.transform_sq
|
|||
|
||||
This method will return a list of SQL statements that should be executed to implement the change. You can then make modifications to that SQL - or add additional SQL statements - before executing it yourself.
|
||||
|
||||
.. _python_api_transform_foreign_keys_transactions:
|
||||
|
||||
Foreign keys and transactions
|
||||
-----------------------------
|
||||
|
||||
Because ``.transform()`` drops the old table, running it with ``PRAGMA foreign_keys`` enabled could fire ``ON DELETE`` actions on any tables that reference it - an inbound ``ON DELETE CASCADE`` foreign key would silently delete those referencing rows. To prevent this, ``.transform()`` turns ``PRAGMA foreign_keys`` off for the duration of the operation and restores it afterwards, running ``PRAGMA foreign_key_check`` before committing.
|
||||
|
||||
``PRAGMA foreign_keys`` cannot be changed inside a transaction, so this protection is impossible if you call ``.transform()`` while a transaction is already open - for example inside a ``with db.atomic():`` block or after ``db.begin()``. If ``PRAGMA foreign_keys`` is on and another table references the table being transformed with a destructive ``ON DELETE`` action - ``CASCADE``, ``SET NULL`` or ``SET DEFAULT`` - the method will refuse to run and raise a ``sqlite_utils.db.TransactionError``:
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
from sqlite_utils.db import TransactionError
|
||||
|
||||
try:
|
||||
with db.atomic():
|
||||
db["authors"].transform(types={"id": str})
|
||||
except TransactionError as ex:
|
||||
print("Could not transform in transaction:", ex)
|
||||
|
||||
To transform such a table either call ``.transform()`` outside of the transaction, or execute ``PRAGMA foreign_keys = off`` before opening it:
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
db.execute("PRAGMA foreign_keys = off")
|
||||
with db.atomic():
|
||||
db["authors"].transform(types={"id": str})
|
||||
db.execute("PRAGMA foreign_keys = on")
|
||||
|
||||
Tables referenced by foreign keys without a destructive action (the default ``NO ACTION``, or ``RESTRICT``) can still be transformed inside a transaction - sqlite-utils uses ``PRAGMA defer_foreign_keys`` to postpone the foreign key checks until the transaction commits.
|
||||
|
||||
.. _python_api_extract:
|
||||
|
||||
Extracting columns into a separate table
|
||||
|
|
|
|||
|
|
@ -2522,6 +2522,11 @@ class Table(Queryable):
|
|||
|
||||
See :ref:`python_api_transform` for full details.
|
||||
|
||||
Raises :py:class:`sqlite_utils.db.TransactionError` if called while a
|
||||
transaction is open with ``PRAGMA foreign_keys`` enabled and the table
|
||||
is referenced by foreign keys with destructive ``ON DELETE`` actions -
|
||||
see :ref:`python_api_transform_foreign_keys_transactions`.
|
||||
|
||||
:param types: Columns that should have their type changed, for example ``{"weight": float}``
|
||||
:param rename: Columns to rename, for example ``{"headline": "title"}``
|
||||
:param drop: Columns to drop
|
||||
|
|
@ -2566,6 +2571,36 @@ class Table(Queryable):
|
|||
should_defer_foreign_keys = (
|
||||
pragma_foreign_keys_was_on and already_in_transaction
|
||||
)
|
||||
if should_defer_foreign_keys:
|
||||
# PRAGMA foreign_keys is a no-op inside a transaction, and
|
||||
# defer_foreign_keys only defers violation checks, not ON DELETE
|
||||
# actions - so dropping the old table would still fire destructive
|
||||
# actions on any tables that reference it. Refuse rather than
|
||||
# silently modify or delete those rows.
|
||||
destructive_fks = [
|
||||
(table.name, fk)
|
||||
for table in self.db.tables
|
||||
for fk in table.foreign_keys
|
||||
if fk.other_table == self.name
|
||||
and fk.on_delete in ("CASCADE", "SET NULL", "SET DEFAULT")
|
||||
]
|
||||
if destructive_fks:
|
||||
raise TransactionError(
|
||||
"Cannot transform table {table} while a transaction is open: "
|
||||
"PRAGMA foreign_keys cannot be changed inside a transaction, "
|
||||
"and the table is referenced by foreign keys with ON DELETE "
|
||||
"actions that would fire when the old table is dropped: "
|
||||
"{fks}. Call transform() outside of the transaction, or "
|
||||
'execute "PRAGMA foreign_keys = off" before opening it.'.format(
|
||||
table=self.name,
|
||||
fks=", ".join(
|
||||
"{}.{} (ON DELETE {})".format(
|
||||
table_name, ", ".join(fk.columns), fk.on_delete
|
||||
)
|
||||
for table_name, fk in destructive_fks
|
||||
),
|
||||
)
|
||||
)
|
||||
defer_foreign_keys_was_on = False
|
||||
try:
|
||||
if should_disable_foreign_keys:
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import sqlite3
|
||||
|
||||
from sqlite_utils.db import ForeignKey, TransformError
|
||||
from sqlite_utils.db import ForeignKey, TransactionError, TransformError
|
||||
from sqlite_utils.utils import OperationalError
|
||||
import pytest
|
||||
|
||||
|
|
@ -469,6 +469,128 @@ def test_transform_on_delete_cascade_does_not_delete_records(
|
|||
assert fresh_db.conn.execute("PRAGMA foreign_keys").fetchone()[0]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("on_delete", ["CASCADE", "SET NULL", "SET DEFAULT", "cascade"])
|
||||
def test_transform_in_transaction_refuses_destructive_on_delete(fresh_db, on_delete):
|
||||
# PRAGMA foreign_keys is a no-op inside a transaction, so transforming a
|
||||
# table referenced by ON DELETE CASCADE / SET NULL / SET DEFAULT foreign
|
||||
# keys inside an open transaction would fire those actions when the old
|
||||
# table is dropped - transform() should refuse instead
|
||||
fresh_db.conn.execute("PRAGMA foreign_keys=ON")
|
||||
fresh_db.executescript("""
|
||||
CREATE TABLE authors (id INTEGER PRIMARY KEY, name TEXT);
|
||||
CREATE TABLE books (
|
||||
id INTEGER PRIMARY KEY,
|
||||
title TEXT,
|
||||
author_id INTEGER REFERENCES authors(id) ON DELETE {}
|
||||
);
|
||||
""".format(on_delete))
|
||||
fresh_db["authors"].insert({"id": 1, "name": "Ursula K. Le Guin"})
|
||||
fresh_db["books"].insert({"id": 1, "title": "The Dispossessed", "author_id": 1})
|
||||
previous_schema = fresh_db["authors"].schema
|
||||
with fresh_db.atomic():
|
||||
with pytest.raises(TransactionError) as excinfo:
|
||||
fresh_db["authors"].transform(rename={"name": "author_name"})
|
||||
message = str(excinfo.value)
|
||||
assert "books" in message
|
||||
assert "ON DELETE {}".format(on_delete.upper()) in message
|
||||
# Nothing should have changed
|
||||
assert fresh_db["authors"].schema == previous_schema
|
||||
assert list(fresh_db["books"].rows) == [
|
||||
{"id": 1, "title": "The Dispossessed", "author_id": 1}
|
||||
]
|
||||
assert fresh_db.conn.execute("PRAGMA foreign_keys").fetchone()[0]
|
||||
|
||||
|
||||
def test_transform_in_transaction_refuses_self_referential_cascade(fresh_db):
|
||||
# The copied table carries a foreign key referencing the original table
|
||||
# name, so a self-referential cascade would wipe the copy too
|
||||
fresh_db.conn.execute("PRAGMA foreign_keys=ON")
|
||||
fresh_db.executescript("""
|
||||
CREATE TABLE categories (
|
||||
id INTEGER PRIMARY KEY,
|
||||
name TEXT,
|
||||
parent_id INTEGER REFERENCES categories(id) ON DELETE CASCADE
|
||||
);
|
||||
""")
|
||||
fresh_db["categories"].insert_all(
|
||||
[
|
||||
{"id": 1, "name": "Fiction", "parent_id": None},
|
||||
{"id": 2, "name": "Science Fiction", "parent_id": 1},
|
||||
]
|
||||
)
|
||||
with fresh_db.atomic():
|
||||
with pytest.raises(TransactionError) as excinfo:
|
||||
fresh_db["categories"].transform(rename={"name": "title"})
|
||||
assert "categories" in str(excinfo.value)
|
||||
assert fresh_db["categories"].count == 2
|
||||
|
||||
|
||||
def test_transform_in_transaction_allowed_with_no_action_foreign_key(fresh_db):
|
||||
# An inbound foreign key without a destructive ON DELETE action is safe
|
||||
# inside a transaction thanks to PRAGMA defer_foreign_keys
|
||||
fresh_db.conn.execute("PRAGMA foreign_keys=ON")
|
||||
fresh_db.executescript("""
|
||||
CREATE TABLE authors (id INTEGER PRIMARY KEY, name TEXT);
|
||||
CREATE TABLE books (
|
||||
id INTEGER PRIMARY KEY,
|
||||
title TEXT,
|
||||
author_id INTEGER REFERENCES authors(id)
|
||||
);
|
||||
""")
|
||||
fresh_db["authors"].insert({"id": 1, "name": "Ursula K. Le Guin"})
|
||||
fresh_db["books"].insert({"id": 1, "title": "The Dispossessed", "author_id": 1})
|
||||
with fresh_db.atomic():
|
||||
fresh_db["authors"].transform(rename={"name": "author_name"})
|
||||
assert list(fresh_db["authors"].rows) == [
|
||||
{"id": 1, "author_name": "Ursula K. Le Guin"}
|
||||
]
|
||||
assert list(fresh_db["books"].rows) == [
|
||||
{"id": 1, "title": "The Dispossessed", "author_id": 1}
|
||||
]
|
||||
assert fresh_db.conn.execute("PRAGMA foreign_keys").fetchone()[0]
|
||||
|
||||
|
||||
def test_transform_in_transaction_allowed_for_child_table(fresh_db):
|
||||
# The table being transformed only has an outbound foreign key - dropping
|
||||
# it fires no ON DELETE actions, so this is allowed inside a transaction
|
||||
fresh_db.conn.execute("PRAGMA foreign_keys=ON")
|
||||
fresh_db.executescript("""
|
||||
CREATE TABLE authors (id INTEGER PRIMARY KEY, name TEXT);
|
||||
CREATE TABLE books (
|
||||
id INTEGER PRIMARY KEY,
|
||||
title TEXT,
|
||||
author_id INTEGER REFERENCES authors(id) ON DELETE CASCADE
|
||||
);
|
||||
""")
|
||||
fresh_db["authors"].insert({"id": 1, "name": "Ursula K. Le Guin"})
|
||||
fresh_db["books"].insert({"id": 1, "title": "The Dispossessed", "author_id": 1})
|
||||
with fresh_db.atomic():
|
||||
fresh_db["books"].transform(rename={"title": "book_title"})
|
||||
assert list(fresh_db["books"].rows) == [
|
||||
{"id": 1, "book_title": "The Dispossessed", "author_id": 1}
|
||||
]
|
||||
|
||||
|
||||
def test_transform_in_transaction_allowed_with_foreign_keys_off(fresh_db):
|
||||
# With PRAGMA foreign_keys off (the default) no cascades can fire, so
|
||||
# transform inside a transaction is safe even with a CASCADE schema
|
||||
fresh_db.executescript("""
|
||||
CREATE TABLE authors (id INTEGER PRIMARY KEY, name TEXT);
|
||||
CREATE TABLE books (
|
||||
id INTEGER PRIMARY KEY,
|
||||
title TEXT,
|
||||
author_id INTEGER REFERENCES authors(id) ON DELETE CASCADE
|
||||
);
|
||||
""")
|
||||
fresh_db["authors"].insert({"id": 1, "name": "Ursula K. Le Guin"})
|
||||
fresh_db["books"].insert({"id": 1, "title": "The Dispossessed", "author_id": 1})
|
||||
with fresh_db.atomic():
|
||||
fresh_db["authors"].transform(rename={"name": "author_name"})
|
||||
assert list(fresh_db["books"].rows) == [
|
||||
{"id": 1, "title": "The Dispossessed", "author_id": 1}
|
||||
]
|
||||
|
||||
|
||||
def test_transform_add_foreign_keys_from_scratch(fresh_db):
|
||||
_add_country_city_continent(fresh_db)
|
||||
fresh_db["places"].insert(_CAVEAU)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue