diff --git a/docs/changelog.rst b/docs/changelog.rst index 4c868f4..5b9355f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,13 +4,6 @@ Changelog =========== -.. _v4_1_1: - -4.1.1 (2026-07-12) ------------------- - -- ``table.transform()`` now raises a ``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 - ``CASCADE``, ``SET NULL`` or ``SET DEFAULT``. The pragma cannot be changed inside a transaction, so previously dropping the old table as part of the transform could fire those actions and silently delete or modify referencing rows. See :ref:`python_api_transform_foreign_keys_transactions` for details and workarounds. (:issue:`794`) -- The :ref:`CLI ` and :ref:`Python API ` documentation now cross-reference each other: CLI sections link to the equivalent Python API functionality and Python API sections link back to the corresponding CLI command. (:issue:`791`) .. _v4_1: 4.1 (2026-07-11) diff --git a/docs/python-api.rst b/docs/python-api.rst index 43b734d..1ed238e 100644 --- a/docs/python-api.rst +++ b/docs/python-api.rst @@ -434,10 +434,9 @@ 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. -Some related safeguards to be aware of: +Two 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: @@ -1997,36 +1996,6 @@ 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 diff --git a/pyproject.toml b/pyproject.toml index 003322c..971f5a0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sqlite-utils" -version = "4.1.1" +version = "4.1" description = "CLI tool and Python library for manipulating SQLite databases" readme = { file = "README.md", content-type = "text/markdown" } authors = [ diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index e97b7d9..d709fb9 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -2522,11 +2522,6 @@ 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 @@ -2571,36 +2566,6 @@ 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: diff --git a/tests/test_transform.py b/tests/test_transform.py index 362f1ca..3cc5ba6 100644 --- a/tests/test_transform.py +++ b/tests/test_transform.py @@ -1,6 +1,6 @@ import sqlite3 -from sqlite_utils.db import ForeignKey, TransactionError, TransformError +from sqlite_utils.db import ForeignKey, TransformError from sqlite_utils.utils import OperationalError import pytest @@ -469,128 +469,6 @@ 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)