[5.1.x] Fixed #34856 -- Fixed references to index_together in historical migrations.

systemallica · sarahboyce · commit 2ee6ca6d35f6 · 2024-12-17T10:01:51.000+01:00
While AlterUniqueTogether has been documented to be still allowed in historical migrations for the foreseeable future it has been crashing since 2abf417 was merged because the latter removed support for Meta.index_together which the migration framework uses to render models to perform schema changes. CreateModel(options["unique_together"]) was also affected. Refs #27236. Co-authored-by: Simon Charette <charette.s@gmail.com> Backport of b44efdf from main.
diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py
@@ -95,6 +95,17 @@ def database_forwards(self, app_label, schema_editor, from_state, to_state):
         model = to_state.apps.get_model(app_label, self.name)
         if self.allow_migrate_model(schema_editor.connection.alias, model):
             schema_editor.create_model(model)
+            # While the `index_together` option has been deprecated some
+            # historical migrations might still have references to them.
+            # This can be moved to the schema editor once it's adapted to
+            # from model states instead of rendered models (#29898).
+            to_model_state = to_state.models[app_label, self.name_lower]
+            if index_together := to_model_state.options.get("index_together"):
+                schema_editor.alter_index_together(
+                    model,
+                    set(),
+                    index_together,
+                )
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
         model = from_state.apps.get_model(app_label, self.name)
@@ -668,12 +679,13 @@ def state_forwards(self, app_label, state):
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.name)
         if self.allow_migrate_model(schema_editor.connection.alias, new_model):
-            old_model = from_state.apps.get_model(app_label, self.name)
+            from_model_state = from_state.models[app_label, self.name_lower]
+            to_model_state = to_state.models[app_label, self.name_lower]
             alter_together = getattr(schema_editor, "alter_%s" % self.option_name)
             alter_together(
                 new_model,
-                getattr(old_model._meta, self.option_name, set()),
-                getattr(new_model._meta, self.option_name, set()),
+                from_model_state.options.get(self.option_name) or set(),
+                to_model_state.options.get(self.option_name) or set(),
             )
 
     def database_backwards(self, app_label, schema_editor, from_state, to_state):
diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py
@@ -310,13 +310,14 @@ def rename_field(self, app_label, model_name, old_name, new_name):
                         for from_field_name in from_fields
                     ]
                 )
-        # Fix unique_together to refer to the new field.
+        # Fix index/unique_together to refer to the new field.
         options = model_state.options
-        if "unique_together" in options:
-            options["unique_together"] = [
-                [new_name if n == old_name else n for n in together]
-                for together in options["unique_together"]
-            ]
+        for option in ("index_together", "unique_together"):
+            if option in options:
+                options[option] = [
+                    [new_name if n == old_name else n for n in together]
+                    for together in options[option]
+                ]
         # Fix to_fields to refer to the new field.
         delay = True
         references = get_references(self, model_key, (old_name, found))
@@ -931,7 +932,11 @@ def clone(self):
     def render(self, apps):
         """Create a Model object from our current state into the given apps."""
         # First, make a Meta object
-        meta_contents = {"app_label": self.app_label, "apps": apps, **self.options}
+        meta_options = {**self.options}
+        # Prune index_together from options as it's no longer an allowed meta
+        # attribute.
+        meta_options.pop("index_together", None)
+        meta_contents = {"app_label": self.app_label, "apps": apps, **meta_options}
         meta = type("Meta", (), meta_contents)
         # Then, work out our bases
         try:
diff --git a/docs/releases/5.1.5.txt b/docs/releases/5.1.5.txt
@@ -9,4 +9,5 @@ Django 5.1.5 fixes several bugs in 5.1.4.
 Bugfixes
 ========
 
-* ...
+* Fixed a crash when applying migrations with references to the removed
+  ``Meta.index_together`` option (:ticket:`34856`).
diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py
@@ -3329,6 +3329,52 @@ def test_rename_field_unique_together(self):
         self.assertColumnExists("test_rnflut_pony", "pink")
         self.assertColumnNotExists("test_rnflut_pony", "blue")
 
+    def test_rename_field_index_together(self):
+        app_label = "test_rnflit"
+        operations = [
+            migrations.CreateModel(
+                "Pony",
+                fields=[
+                    ("id", models.AutoField(primary_key=True)),
+                    ("pink", models.IntegerField(default=3)),
+                    ("weight", models.FloatField()),
+                ],
+                options={
+                    "index_together": [("weight", "pink")],
+                },
+            ),
+        ]
+        project_state = self.apply_operations(app_label, ProjectState(), operations)
+
+        operation = migrations.RenameField("Pony", "pink", "blue")
+        new_state = project_state.clone()
+        operation.state_forwards("test_rnflit", new_state)
+        self.assertIn("blue", new_state.models["test_rnflit", "pony"].fields)
+        self.assertNotIn("pink", new_state.models["test_rnflit", "pony"].fields)
+        # index_together has the renamed column.
+        self.assertIn(
+            "blue", new_state.models["test_rnflit", "pony"].options["index_together"][0]
+        )
+        self.assertNotIn(
+            "pink", new_state.models["test_rnflit", "pony"].options["index_together"][0]
+        )
+
+        # Rename field.
+        self.assertColumnExists("test_rnflit_pony", "pink")
+        self.assertColumnNotExists("test_rnflit_pony", "blue")
+        with connection.schema_editor() as editor:
+            operation.database_forwards("test_rnflit", editor, project_state, new_state)
+        self.assertColumnExists("test_rnflit_pony", "blue")
+        self.assertColumnNotExists("test_rnflit_pony", "pink")
+        # The index constraint has been ported over.
+        self.assertIndexExists("test_rnflit_pony", ["weight", "blue"])
+        # Reversal.
+        with connection.schema_editor() as editor:
+            operation.database_backwards(
+                "test_rnflit", editor, new_state, project_state
+            )
+        self.assertIndexExists("test_rnflit_pony", ["weight", "pink"])
+
     def test_rename_field_with_db_column(self):
         project_state = self.apply_operations(
             "test_rfwdbc",
@@ -3822,6 +3868,63 @@ def test_rename_index_arguments(self):
         with self.assertRaisesMessage(ValueError, msg):
             migrations.RenameIndex("Pony", new_name="new_idx_name")
 
+    def test_rename_index_unnamed_index(self):
+        app_label = "test_rninui"
+        operations = [
+            migrations.CreateModel(
+                "Pony",
+                fields=[
+                    ("id", models.AutoField(primary_key=True)),
+                    ("pink", models.IntegerField(default=3)),
+                    ("weight", models.FloatField()),
+                ],
+                options={
+                    "index_together": [("weight", "pink")],
+                },
+            ),
+        ]
+        project_state = self.apply_operations(app_label, ProjectState(), operations)
+        table_name = app_label + "_pony"
+        self.assertIndexNameNotExists(table_name, "new_pony_test_idx")
+        operation = migrations.RenameIndex(
+            "Pony", new_name="new_pony_test_idx", old_fields=("weight", "pink")
+        )
+        self.assertEqual(
+            operation.describe(),
+            "Rename unnamed index for ('weight', 'pink') on Pony to new_pony_test_idx",
+        )
+        self.assertEqual(
+            operation.migration_name_fragment,
+            "rename_pony_weight_pink_new_pony_test_idx",
+        )
+        new_state = project_state.clone()
+        operation.state_forwards(app_label, new_state)
+        # Rename index.
+        with connection.schema_editor() as editor:
+            operation.database_forwards(app_label, editor, project_state, new_state)
+        self.assertIndexNameExists(table_name, "new_pony_test_idx")
+        # Reverse is a no-op.
+        with connection.schema_editor() as editor, self.assertNumQueries(0):
+            operation.database_backwards(app_label, editor, new_state, project_state)
+        self.assertIndexNameExists(table_name, "new_pony_test_idx")
+        # Reapply, RenameIndex operation is a noop when the old and new name
+        # match.
+        with connection.schema_editor() as editor:
+            operation.database_forwards(app_label, editor, new_state, project_state)
+        self.assertIndexNameExists(table_name, "new_pony_test_idx")
+        # Deconstruction.
+        definition = operation.deconstruct()
+        self.assertEqual(definition[0], "RenameIndex")
+        self.assertEqual(definition[1], [])
+        self.assertEqual(
+            definition[2],
+            {
+                "model_name": "Pony",
+                "new_name": "new_pony_test_idx",
+                "old_fields": ("weight", "pink"),
+            },
+        )
+
     def test_rename_index_unknown_unnamed_index(self):
         app_label = "test_rninuui"
         project_state = self.set_up_test_model(app_label)
@@ -3892,6 +3995,33 @@ def test_rename_index_state_forwards(self):
         self.assertIsNot(old_model, new_model)
         self.assertEqual(new_model._meta.indexes[0].name, "new_pony_pink_idx")
 
+    def test_rename_index_state_forwards_unnamed_index(self):
+        app_label = "test_rnidsfui"
+        operations = [
+            migrations.CreateModel(
+                "Pony",
+                fields=[
+                    ("id", models.AutoField(primary_key=True)),
+                    ("pink", models.IntegerField(default=3)),
+                    ("weight", models.FloatField()),
+                ],
+                options={
+                    "index_together": [("weight", "pink")],
+                },
+            ),
+        ]
+        project_state = self.apply_operations(app_label, ProjectState(), operations)
+        old_model = project_state.apps.get_model(app_label, "Pony")
+        new_state = project_state.clone()
+
+        operation = migrations.RenameIndex(
+            "Pony", new_name="new_pony_pink_idx", old_fields=("weight", "pink")
+        )
+        operation.state_forwards(app_label, new_state)
+        new_model = new_state.apps.get_model(app_label, "Pony")
+        self.assertIsNot(old_model, new_model)
+        self.assertEqual(new_model._meta.indexes[0].name, "new_pony_pink_idx")
+
     @skipUnlessDBFeature("supports_expression_indexes")
     def test_add_func_index(self):
         app_label = "test_addfuncin"
@@ -4011,6 +4141,58 @@ def test_alter_field_with_index(self):
         # Ensure the index is still there
         self.assertIndexExists("test_alflin_pony", ["pink"])
 
+    def test_alter_index_together(self):
+        """
+        Tests the AlterIndexTogether operation.
+        """
+        project_state = self.set_up_test_model("test_alinto")
+        # Test the state alteration
+        operation = migrations.AlterIndexTogether("Pony", [("pink", "weight")])
+        self.assertEqual(
+            operation.describe(), "Alter index_together for Pony (1 constraint(s))"
+        )
+        self.assertEqual(
+            operation.migration_name_fragment,
+            "alter_pony_index_together",
+        )
+        new_state = project_state.clone()
+        operation.state_forwards("test_alinto", new_state)
+        self.assertEqual(
+            len(
+                project_state.models["test_alinto", "pony"].options.get(
+                    "index_together", set()
+                )
+            ),
+            0,
+        )
+        self.assertEqual(
+            len(
+                new_state.models["test_alinto", "pony"].options.get(
+                    "index_together", set()
+                )
+            ),
+            1,
+        )
+        # Make sure there's no matching index
+        self.assertIndexNotExists("test_alinto_pony", ["pink", "weight"])
+        # Test the database alteration
+        with connection.schema_editor() as editor:
+            operation.database_forwards("test_alinto", editor, project_state, new_state)
+        self.assertIndexExists("test_alinto_pony", ["pink", "weight"])
+        # And test reversal
+        with connection.schema_editor() as editor:
+            operation.database_backwards(
+                "test_alinto", editor, new_state, project_state
+            )
+        self.assertIndexNotExists("test_alinto_pony", ["pink", "weight"])
+        # And deconstruction
+        definition = operation.deconstruct()
+        self.assertEqual(definition[0], "AlterIndexTogether")
+        self.assertEqual(definition[1], [])
+        self.assertEqual(
+            definition[2], {"name": "Pony", "index_together": {("pink", "weight")}}
+        )
+
     def test_alter_index_together_remove(self):
         operation = migrations.AlterIndexTogether("Pony", None)
         self.assertEqual(
@@ -4021,6 +4203,29 @@ def test_alter_index_together_remove(self):
             "~ Alter index_together for Pony (0 constraint(s))",
         )
 
+    @skipUnlessDBFeature("allows_multiple_constraints_on_same_fields")
+    def test_alter_index_together_remove_with_unique_together(self):
+        app_label = "test_alintoremove_wunto"
+        table_name = "%s_pony" % app_label
+        project_state = self.set_up_test_model(app_label, unique_together=True)
+        self.assertUniqueConstraintExists(table_name, ["pink", "weight"])
+        # Add index together.
+        new_state = project_state.clone()
+        operation = migrations.AlterIndexTogether("Pony", [("pink", "weight")])
+        operation.state_forwards(app_label, new_state)
+        with connection.schema_editor() as editor:
+            operation.database_forwards(app_label, editor, project_state, new_state)
+        self.assertIndexExists(table_name, ["pink", "weight"])
+        # Remove index together.
+        project_state = new_state
+        new_state = project_state.clone()
+        operation = migrations.AlterIndexTogether("Pony", set())
+        operation.state_forwards(app_label, new_state)
+        with connection.schema_editor() as editor:
+            operation.database_forwards(app_label, editor, project_state, new_state)
+        self.assertIndexNotExists(table_name, ["pink", "weight"])
+        self.assertUniqueConstraintExists(table_name, ["pink", "weight"])
+
     def test_add_constraint(self):
         project_state = self.set_up_test_model("test_addconstraint")
         gt_check = models.Q(pink__gt=2)
