From 5b61530965df0d56a09e71f29cc970b4ea3e43d4 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 5 Jul 2026 15:42:41 -0700 Subject: [PATCH] db.add_foreign_keys() no longer drops ON DELETE/ON UPDATE actions ForeignKey objects passed to db.add_foreign_keys() were flattened to plain (table, column, other_table, other_column) tuples before being handed to transform(), silently discarding their on_delete and on_update actions. The method now carries ForeignKey objects through to transform() intact - tuple inputs are converted to ForeignKey objects up front, which also simplifies the validation loop. Co-Authored-By: Claude Fable 5 --- sqlite_utils/db.py | 42 ++++++++++++++++++++++---------------- tests/test_foreign_keys.py | 35 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 18 deletions(-) diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index 1c06b62..7379859 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -1630,17 +1630,12 @@ class Database: "(table, column, other_table, other_column)" ) - foreign_keys_to_create: List[Tuple[str, Any, str, Any]] = [] + foreign_keys_to_create: List[ForeignKey] = [] # Verify that all tables and columns exist for fk in foreign_keys: if isinstance(fk, ForeignKey): - table, columns, other_table, other_columns = ( - fk.table, - fk.columns, - fk.other_table, - fk.other_columns, - ) + fk_object = fk else: table, column_or_columns, other_table, other_column_or_columns = fk # Compound foreign keys use tuples of columns @@ -1654,6 +1649,24 @@ class Database: if isinstance(other_column_or_columns, str) else tuple(other_column_or_columns) ) + if len(columns) == 1: + fk_object = ForeignKey( + table, columns[0], other_table, other_columns[0] + ) + else: + fk_object = ForeignKey( + table, + None, + other_table, + None, + columns=columns, + other_columns=other_columns, + is_compound=True, + ) + table = fk_object.table + columns = fk_object.columns + other_table = fk_object.other_table + other_columns = fk_object.other_columns if not self.table(table).exists(): raise AlterError("No such table: {}".format(table)) table_obj = self.table(table) @@ -1680,19 +1693,12 @@ class Database: and fk.other_table == other_table and fk.other_columns == other_columns ): - if len(columns) == 1: - foreign_keys_to_create.append( - (table, columns[0], other_table, other_columns[0]) - ) - else: - foreign_keys_to_create.append( - (table, columns, other_table, other_columns) - ) + foreign_keys_to_create.append(fk_object) # Group them by table - by_table: Dict[str, List] = {} - for fk in foreign_keys_to_create: - by_table.setdefault(fk[0], []).append(fk) + by_table: Dict[str, List[ForeignKey]] = {} + for fk_object in foreign_keys_to_create: + by_table.setdefault(fk_object.table, []).append(fk_object) for table, fks in by_table.items(): self.table(table).transform(add_foreign_keys=fks) diff --git a/tests/test_foreign_keys.py b/tests/test_foreign_keys.py index d1bc498..b22638a 100644 --- a/tests/test_foreign_keys.py +++ b/tests/test_foreign_keys.py @@ -464,3 +464,38 @@ def test_foreign_key_normalizes_list_columns_to_tuples(): ) assert fk.columns == ("campus_name", "dept_code") assert fk.other_columns == ("campus_name", "dept_code") + + +def test_add_foreign_keys_preserves_actions(fresh_db): + # https://github.com/simonw/sqlite-utils/issues/594 review finding: + # ForeignKey objects passed to db.add_foreign_keys() were flattened + # to plain tuples, losing on_delete/on_update + fresh_db["authors"].insert({"id": 1}, pk="id") + fresh_db["books"].insert({"id": 1, "author_id": 1}, pk="id") + fresh_db.add_foreign_keys( + [ForeignKey("books", "author_id", "authors", "id", on_delete="CASCADE")] + ) + fk = fresh_db["books"].foreign_keys[0] + assert fk.on_delete == "CASCADE" + assert "ON DELETE CASCADE" in fresh_db["books"].schema + + +def test_add_foreign_keys_preserves_actions_compound(courses_db): + courses_db.add_foreign_keys( + [ + ForeignKey( + table="courses", + column=None, + other_table="departments", + other_column=None, + columns=("campus_name", "dept_code"), + other_columns=("campus_name", "dept_code"), + is_compound=True, + on_delete="CASCADE", + ) + ] + ) + fk = courses_db["courses"].foreign_keys[0] + assert fk.is_compound is True + assert fk.on_delete == "CASCADE" + assert "ON DELETE CASCADE" in courses_db["courses"].schema