foreign_keys= accepts mixed ForeignKey objects, tuples and strings again

resolve_foreign_keys() used all-or-nothing isinstance checks, so mixing
ForeignKey objects with tuples raised "foreign_keys= should be a list of
tuples" - a regression from 3.x, where ForeignKey was a namedtuple and
passed the tuple check. Each item is now normalized individually, which
also allows bare column-name strings in a mixed list.

Refs https://github.com/simonw/sqlite-utils/issues/769#issuecomment-4900034150

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

docs/changelog.rst

@@ -19,6 +19,7 @@ Unreleased
 - Fixed a bug where a failed write statement executed with ``db.execute()`` left the driver's implicit transaction open. Every subsequent write then joined that phantom transaction, which nothing committed, so their work was silently rolled back when the connection was closed. The implicit transaction opened by a failed statement is now rolled back before the exception is raised. A failed write inside a transaction opened with ``db.begin()`` or ``db.atomic()`` leaves that transaction open and untouched, as before.
 - Fixed a bug where transaction-control statements prefixed with an empty statement - ``db.query("; COMMIT")`` - or a UTF-8 byte order mark slipped past the check that rejects them, committing the caller's open transaction before raising a confusing ``OperationalError``. The keyword scanner used by ``db.query()`` and ``db.execute()`` now skips leading ``;`` and byte order marks, matching what the ``sqlite3`` driver tolerates before the first token, so these statements are rejected with a ``ValueError`` without being executed. The same fix means ``db.execute("; BEGIN")`` no longer auto-commits the transaction it just opened.
 - Documented a limitation of ``db.query()``: a ``PRAGMA`` statement that returns no rows raises a ``ValueError`` but still takes effect, because PRAGMA statements run outside the savepoint guard used to roll back other rejected statements. Use ``db.execute()`` for row-less PRAGMA statements.
+- The ``foreign_keys=`` argument to ``create()`` and ``insert()`` accepts a mixed list of ``ForeignKey`` objects, tuples and column name strings again. In 4.0 pre-releases mixing ``ForeignKey`` objects with tuples raised a ``ValueError`` - a regression from 3.x, where ``ForeignKey`` was a ``namedtuple`` and passed the tuple checks.
 - ``ForeignKey`` objects are hashable again. The 4.0 change from ``namedtuple`` to dataclass accidentally made them unhashable, breaking patterns like ``set(table.foreign_keys)`` that worked in 3.x. ``ForeignKey`` is now a frozen dataclass - immutable and hashable, like the namedtuple was.
 - Fixed a bug where compound primary key columns were returned in table column order instead of ``PRIMARY KEY`` declaration order. For a table declared as ``CREATE TABLE other (b TEXT, a TEXT, PRIMARY KEY (a, b))`` an implicit ``FOREIGN KEY (x, y) REFERENCES other`` was introspected as referencing ``(b, a)`` when SQLite resolves it as ``(a, b)`` - running ``transform()`` on such a table then rewrote the schema with the inverted column order, silently reversing the meaning of the constraint and causing foreign key errors on valid data. ``table.pks``, compound foreign key guessing and ``transform()`` now all use the primary key declaration order, and ``transform()`` no longer reorders a compound ``PRIMARY KEY (b, a)`` into table column order.
 

sqlite_utils/db.py

@@ -1238,21 +1238,23 @@ class Database:
             (("campus_name", "dept_code"), "departments", ("campus_name", "dept_code"))
         """
         table = self.table(name)
-        if all(isinstance(fk, ForeignKey) for fk in foreign_keys):
-            return cast(List[ForeignKey], foreign_keys)
-        if all(isinstance(fk, str) for fk in foreign_keys):
-            # It's a list of columns
-            fks = []
-            for column in foreign_keys:
-                column = cast(str, column)
-                other_table = table.guess_foreign_table(column)
-                other_column = table.guess_foreign_column(other_table)
-                fks.append(ForeignKey(name, column, other_table, other_column))
-            return fks
-        if not all(isinstance(fk, (tuple, list)) for fk in foreign_keys):
-            raise ValueError("foreign_keys= should be a list of tuples")
         fks = []
-        for tuple_or_list in cast(Iterable[Sequence[Any]], foreign_keys):
+        for fk in foreign_keys:
+            if isinstance(fk, ForeignKey):
+                fks.append(fk)
+                continue
+            if isinstance(fk, str):
+                # A bare column name - guess the other table and column
+                other_table = table.guess_foreign_table(fk)
+                other_column = table.guess_foreign_column(other_table)
+                fks.append(ForeignKey(name, fk, other_table, other_column))
+                continue
+            if not isinstance(fk, (tuple, list)):
+                raise ValueError(
+                    "foreign_keys= should be a list of tuples, "
+                    "ForeignKey objects or column name strings"
+                )
+            tuple_or_list = cast(Sequence[Any], fk)
             if len(tuple_or_list) == 4:
                 if tuple_or_list[0] != name:
                     raise ValueError(

tests/test_foreign_keys.py

@@ -610,3 +610,36 @@ def test_foreign_key_equality_and_hash_include_actions():
     assert plain != cascade
     assert len({plain, cascade}) == 2
     assert plain == ForeignKey("c", "pid", "p", "id")
+
+
+def test_create_table_mixed_foreign_keys_list(fresh_db):
+    # 3.x accepted a mix of ForeignKey objects, tuples and bare column
+    # strings in foreign_keys= (ForeignKey was a namedtuple, so it passed
+    # the tuple check) - keep accepting the mix
+    fresh_db["authors"].insert({"id": 1}, pk="id")
+    fresh_db["publishers"].insert({"id": 1}, pk="id")
+    fresh_db["books"].create(
+        {"id": int, "author_id": int, "publisher_id": int},
+        pk="id",
+        foreign_keys=[
+            ForeignKey("books", "author_id", "authors", "id"),
+            ("publisher_id", "publishers", "id"),
+        ],
+    )
+    fks = {fk.column: fk.other_table for fk in fresh_db["books"].foreign_keys}
+    assert fks == {"author_id": "authors", "publisher_id": "publishers"}
+
+
+def test_create_table_mixed_foreign_keys_with_string(fresh_db):
+    fresh_db["authors"].insert({"id": 1}, pk="id")
+    fresh_db["publishers"].insert({"id": 1}, pk="id")
+    fresh_db["books"].create(
+        {"id": int, "author_id": int, "publisher_id": int},
+        pk="id",
+        foreign_keys=[
+            "author_id",  # bare column, table and column guessed
+            ("publisher_id", "publishers", "id"),
+        ],
+    )
+    fks = {fk.column: fk.other_table for fk in fresh_db["books"].foreign_keys}
+    assert fks == {"author_id": "authors", "publisher_id": "publishers"}