Fixed #36878 -- Unified data type for *_together options in ModelState.

Ever since the beginning of Django's migration framework, there's been a
bit of an inconsistency on how index_together and unique_together values
have been stored on the ModelState[^1].

It's only really obvious, when looking at the current code for
`from_model()`[^2] and the `rename_field()` state alteration code[^3].

The problem in the autodetector's detection of the `*_together` options
as raised in the ticket, reinforces the inconsistency[^4]: the old value
is being normalized to a set of tuples, whereas the new value is taken
as-is.

Why this hasn't been caught before, is likely to the fact, that we
never really look at a `to_state` that comes from migration operations
in the autodetector. Instead, in both usages in Django[^5], [^6] the
`to_state` is a `ProjectState.from_apps()`. And that state is
consistently using sets of tuples and not lists of lists.

[^1]: 67dcea711e (diff-5dd147e9e978e645313dd99eab3a7bab1f1cb0a53e256843adb68aeed71e61dcR85-R87)
[^2]: b1ffa9a9d7/django/db/migrations/state.py (L842)
[^3]: b1ffa9a9d7/django/db/migrations/state.py (L340-L345)
[^4]: b1ffa9a9d7/django/db/migrations/autodetector.py (L1757-L1771)
[^5]: 2351c1b12c/django/core/management/commands/makemigrations.py (L215-L219)
[^6]: 2351c1b12c/django/core/management/commands/migrate.py (L329-L332)

django/db/migrations/state.py

@@ -192,9 +192,10 @@ class ProjectState:
     def remove_model_options(self, app_label, model_name, option_name, value_to_remove):
         model_state = self.models[app_label, model_name]
         if objs := model_state.options.get(option_name):
-            model_state.options[option_name] = [
-                obj for obj in objs if tuple(obj) != tuple(value_to_remove)
-            ]
+            new_value = [obj for obj in objs if tuple(obj) != tuple(value_to_remove)]
+            if option_name in {"index_together", "unique_together"}:
+                new_value = set(normalize_together(new_value))
+            model_state.options[option_name] = new_value
         self.reload_model(app_label, model_name, delay=True)
 
     def alter_model_managers(self, app_label, model_name, managers):
@@ -339,10 +340,10 @@ class ProjectState:
         options = model_state.options
         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]
+                options[option] = {
+                    tuple(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))

tests/migrations/test_autodetector.py

@@ -5590,6 +5590,51 @@ class AutodetectorTests(BaseAutodetectorTests):
             preserve_default=True,
         )
 
+    def test_does_not_crash_after_rename_on_unique_together(self):
+        fields = ("first", "second")
+        before = self.make_project_state(
+            [
+                ModelState(
+                    "app",
+                    "Foo",
+                    [
+                        ("id", models.AutoField(primary_key=True)),
+                        ("first", models.IntegerField()),
+                        ("second", models.IntegerField()),
+                    ],
+                    options={"unique_together": {fields}},
+                ),
+            ]
+        )
+        after = before.clone()
+        after.rename_field("app", "foo", "first", "first_renamed")
+
+        changes = MigrationAutodetector(
+            before, after, MigrationQuestioner({"ask_rename": True})
+        )._detect_changes()
+
+        self.assertNumberMigrations(changes, "app", 1)
+        self.assertOperationTypes(
+            changes, "app", 0, ["RenameField", "AlterUniqueTogether"]
+        )
+        self.assertOperationAttributes(
+            changes,
+            "app",
+            0,
+            0,
+            model_name="foo",
+            old_name="first",
+            new_name="first_renamed",
+        )
+        self.assertOperationAttributes(
+            changes,
+            "app",
+            0,
+            1,
+            name="foo",
+            unique_together={("first_renamed", "second")},
+        )
+
 
 class MigrationSuggestNameTests(SimpleTestCase):
     def test_no_operations(self):

tests/migrations/test_base.py

@@ -290,14 +290,13 @@ class OperationTestBase(MigrationTestBase):
     ):
         """Creates a test model state and database table."""
         # Make the "current" state.
-        model_options = {
-            "swappable": "TEST_SWAP_MODEL",
-            "unique_together": [["pink", "weight"]] if unique_together else [],
-        }
+        model_options = {"swappable": "TEST_SWAP_MODEL"}
         if options:
             model_options["permissions"] = [("can_groom", "Can groom")]
         if db_table:
             model_options["db_table"] = db_table
+        if unique_together:
+            model_options["unique_together"] = {("pink", "weight")}
         operations = [
             migrations.CreateModel(
                 "Pony",

tests/migrations/test_operations.py

@@ -3336,11 +3336,11 @@ class OperationTests(OperationTestBase):
         # unique_together has the renamed column.
         self.assertIn(
             "blue",
-            new_state.models["test_rnflut", "pony"].options["unique_together"][0],
+            list(new_state.models["test_rnflut", "pony"].options["unique_together"])[0],
         )
         self.assertNotIn(
             "pink",
-            new_state.models["test_rnflut", "pony"].options["unique_together"][0],
+            list(new_state.models["test_rnflut", "pony"].options["unique_together"])[0],
         )
         # Rename field.
         self.assertColumnExists("test_rnflut_pony", "pink")
@@ -3377,7 +3377,7 @@ class OperationTests(OperationTestBase):
                     ("weight", models.FloatField()),
                 ],
                 options={
-                    "index_together": [("weight", "pink")],
+                    "index_together": {("weight", "pink")},
                 },
             ),
         ]
@@ -3390,10 +3390,12 @@ class OperationTests(OperationTestBase):
         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]
+            "blue",
+            list(new_state.models["test_rnflit", "pony"].options["index_together"])[0],
         )
         self.assertNotIn(
-            "pink", new_state.models["test_rnflit", "pony"].options["index_together"][0]
+            "pink",
+            list(new_state.models["test_rnflit", "pony"].options["index_together"])[0],
         )
 
         # Rename field.
@@ -3952,7 +3954,7 @@ class OperationTests(OperationTestBase):
                     ("weight", models.FloatField()),
                 ],
                 options={
-                    "index_together": [("weight", "pink")],
+                    "index_together": {("weight", "pink")},
                 },
             ),
         ]
@@ -3972,6 +3974,11 @@ class OperationTests(OperationTestBase):
         )
         new_state = project_state.clone()
         operation.state_forwards(app_label, new_state)
+        # Ensure the model state has the correct type for the index_together
+        # option.
+        self.assertIsInstance(
+            new_state.models[app_label, "pony"].options["index_together"], set
+        )
         # Rename index.
         with connection.schema_editor() as editor:
             operation.database_forwards(app_label, editor, project_state, new_state)
@@ -4079,7 +4086,7 @@ class OperationTests(OperationTestBase):
                     ("weight", models.FloatField()),
                 ],
                 options={
-                    "index_together": [("weight", "pink")],
+                    "index_together": {("weight", "pink")},
                 },
             ),
         ]