Automigration silently drops one of two same-named tables in different schemas

piccolo/apps/migrations/auto/diffable_table.py

@@ -266,19 +266,18 @@ def __sub__(self, value: DiffableTable) -> TableDelta:
         )
 
     def __hash__(self) -> int:
-        """
-        We have to return an integer, which is why convert the string this way.
-        """
-        return hash(self.class_name + self.tablename)
+        return hash((self.class_name, self.tablename, self.schema))
 
     def __eq__(self, value) -> bool:
         """
         This is used by sets for uniqueness checks.
         """
         if not isinstance(value, DiffableTable):
             return False
-        return (self.class_name == value.class_name) and (
-            self.tablename == value.tablename
+        return (
+            (self.class_name == value.class_name)
+            and (self.tablename == value.tablename)
+            and (self.schema == value.schema)
         )
 
     def __str__(self):

piccolo/apps/migrations/auto/schema_differ.py

@@ -66,6 +66,10 @@ class ChangeTableSchemaCollection:
     def append(self, change_table_schema: ChangeTableSchema):
         self.collection.append(change_table_schema)
 
+    @property
+    def class_names(self) -> list[str]:
+        return [i.class_name for i in self.collection]
+
 
 @dataclass
 class RenameColumnCollection:
@@ -138,13 +142,22 @@ def check_rename_tables(self) -> RenameTableCollection:
         """
         Work out whether any of the tables were renamed.
         """
-        drop_tables: list[DiffableTable] = list(
-            set(self.schema_snapshot) - set(self.schema)
-        )
+        # Tables which just had their schema changed are handled separately
+        # by ``check_table_schema_changes`` - excluded here so a schema
+        # change alone doesn't get misdetected as a same-named rename.
+        drop_tables: list[DiffableTable] = [
+            i
+            for i in set(self.schema_snapshot) - set(self.schema)
+            if i.class_name
+            not in self.table_schema_changes_collection.class_names
+        ]
 
-        new_tables: list[DiffableTable] = list(
-            set(self.schema) - set(self.schema_snapshot)
-        )
+        new_tables: list[DiffableTable] = [
+            i
+            for i in set(self.schema) - set(self.schema_snapshot)
+            if i.class_name
+            not in self.table_schema_changes_collection.class_names
+        ]
 
         # A mapping of the old table name (i.e. dropped table) to the new
         # table name.
@@ -353,12 +366,16 @@ def create_tables(self) -> AlterStatements:
             set(self.schema) - set(self.schema_snapshot)
         )
 
-        # Remove any which are renames
+        # Remove any which are renames, or just had their schema changed
+        # (both are handled separately, and only match on class_name, so
+        # they're unaffected by the schema-aware equality check above).
         new_tables = [
             i
             for i in new_tables
             if i.class_name
             not in self.rename_tables_collection.new_class_names
+            and i.class_name
+            not in self.table_schema_changes_collection.class_names
         ]
 
         alter_statements = AlterStatements()
@@ -384,12 +401,16 @@ def drop_tables(self) -> AlterStatements:
             set(self.schema_snapshot) - set(self.schema)
         )
 
-        # Remove any which are renames
+        # Remove any which are renames, or just had their schema changed
+        # (both are handled separately, and only match on class_name, so
+        # they're unaffected by the schema-aware equality check above).
         drop_tables = [
             i
             for i in drop_tables
             if i.class_name
             not in self.rename_tables_collection.old_class_names
+            and i.class_name
+            not in self.table_schema_changes_collection.class_names
         ]
 
         alter_statements = AlterStatements()

tests/apps/migrations/auto/test_schema_differ.py

@@ -101,19 +101,22 @@ def test_change_schema(self) -> None:
         """
         Testing changing the schema.
         """
+        name_column = Varchar()
+        name_column._meta.name = "name"
+
         schema: list[DiffableTable] = [
             DiffableTable(
                 class_name="Band",
                 tablename="band",
-                columns=[],
+                columns=[name_column],
                 schema="schema_1",
             )
         ]
         schema_snapshot: list[DiffableTable] = [
             DiffableTable(
                 class_name="Band",
                 tablename="band",
-                columns=[],
+                columns=[name_column],
                 schema=None,
             )
         ]
@@ -132,6 +135,42 @@ def test_change_schema(self) -> None:
         self.assertListEqual(schema_differ.create_tables.statements, [])
         self.assertListEqual(schema_differ.drop_tables.statements, [])
 
+        # A schema change alone shouldn't be misdetected as a table rename
+        # (this table has overlapping columns, which is what the rename
+        # heuristic keys off).
+        self.assertListEqual(schema_differ.rename_tables.statements, [])
+
+    def test_same_table_name_different_schema(self) -> None:
+        """
+        Two tables with the same class_name / tablename, but in different
+        schemas, are different tables - both should be created.
+
+        https://github.com/piccolo-orm/piccolo/issues/1426
+        """
+        schema: list[DiffableTable] = [
+            DiffableTable(
+                class_name="Band", tablename="band", schema="schema_1"
+            ),
+            DiffableTable(
+                class_name="Band", tablename="band", schema="schema_2"
+            ),
+        ]
+        schema_snapshot: list[DiffableTable] = []
+
+        schema_differ = SchemaDiffer(
+            schema=schema, schema_snapshot=schema_snapshot, auto_input="y"
+        )
+
+        create_tables = schema_differ.create_tables
+        self.assertEqual(len(create_tables.statements), 2)
+        self.assertEqual(
+            set(create_tables.statements),
+            {
+                "manager.add_table(class_name='Band', tablename='band', schema='schema_1', columns=None)",  # noqa: E501
+                "manager.add_table(class_name='Band', tablename='band', schema='schema_2', columns=None)",  # noqa: E501
+            },
+        )
+
     def test_add_column(self) -> None:
         """
         Test adding a column to an existing table.