Migration's ModelState has varying type for unique_together and index_together options causing autodetector crash

[Markus Holtermann]

When leveraging the MigrationAutodetector to get the changes between two project states, the following exception is raised (on 5.2, but the same applies to the current master at commit b1ffa9a9d78b0c2c5ad6ed5a1d84e380d5cfd010):

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "src/manage.py", line 67, in <module>
    management.execute_from_command_line(sys.argv)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File ".venv/lib/python3.13/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
    ~~~~~~~~~~~~~~~^^
  File ".venv/lib/python3.13/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File ".venv/lib/python3.13/site-packages/django/core/management/base.py", line 420, in run_from_argv
    self.execute(*args, **cmd_options)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File ".venv/lib/python3.13/site-packages/django/core/management/base.py", line 464, in execute
    output = self.handle(*args, **options)
  File "src/core/management/commands/some_command.py", line 94, in handle
    self._get_migrations_and_operations(section)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "src/core/management/commands/some_command.py", line 238, in _get_migrations_and_operations
    new_migrations = autodetector.changes(self.graph, trim_to_apps={"affiliates"})
  File ".venv/lib/python3.13/site-packages/django/db/migrations/autodetector.py", line 67, in changes
    changes = self._detect_changes(convert_apps, graph)
  File ".venv/lib/python3.13/site-packages/django/db/migrations/autodetector.py", line 213, in _detect_changes
    self.generate_removed_altered_unique_together()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File ".venv/lib/python3.13/site-packages/django/db/migrations/autodetector.py", line 1718, in generate_removed_altered_unique_together
    self._generate_removed_altered_foo_together(operations.AlterUniqueTogether)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv/lib/python3.13/site-packages/django/db/migrations/autodetector.py", line 1699, in _generate_removed_altered_foo_together
    ) in self._get_altered_foo_together_operations(operation.option_name):
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv/lib/python3.13/site-packages/django/db/migrations/autodetector.py", line 1668, in _get_altered_foo_together_operations
    new_value = set(new_value) if new_value else set()
                ~~~^^^^^^^^^^^
TypeError: unhashable type: 'list'

Reason:

The migration ModelState tracks the changes for all options (order_with_respect_to, unique_together, indexes, ...) in its options attribute, which is a simple dict. For most keys inside the dict, the value is just a list of something. However, for unique_together and index_together, the value should be a set of tuples (​see the ModelState.from_model() method).

Unfortunately, there are situations inside the ProjectState's mutation functions (​e.g. rename_field()) where the data type for model_state.options["unique_together"] is changed to list[list[str]]:

        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]
                ]

[Markus Holtermann]

In order to understand where the incorrect setting or the values came from, I applied the following changes to the state.py:

django/db/migrations/state.py

@@ -734 +734 @@
             pass
 
 
+class OptionsDict(dict):
+    def __getitem__(self, key):
+        val = super().__getitem__(key)
+        if "together" in key:
+            print(f"__getitem__({key!r}) = {val!r}")
+        return val
+
+    def __setitem__(self, key, value):
+        if "together" in key:
+            import inspect, textwrap
+
+            print(f"__setitem__({key!r}, {value!r})")
+            if not isinstance(value, set | tuple):
+                calling_frame = inspect.stack(3)[1]
+                code = textwrap.indent("".join(calling_frame.code_context), "    ")
+                print(
+                    f"  {calling_frame.filename}:{calling_frame.lineno} in {calling_frame.function}\n{code}"
+                )
+        return super().__setitem__(key, value)
+
+    def __delitem__(self, key):
+        if "together" in key:
+            print(f"__delitem__({key!r})")
+        return super().__delitem__(key)
+
+
 class ModelState:
     """
     Represent a Django Model. Don't use the actual Model class as it's not
@@ -751 +777 @@
         self.app_label = app_label
         self.name = name
         self.fields = dict(fields)
-        self.options = options or {}
+        self.options = OptionsDict(options or {})
         self.options.setdefault("indexes", [])
         self.options.setdefault("constraints", [])
         self.bases = bases or (models.Model,)
@@ -783 +809 @@
                     "%r doesn't have one." % index
                 )
 
+    def __getattr__(self, name):
+        val = super().__getattr__(name)
+        if (
+            name == "options"
+            and val
+            and ("unique_together" in value or "index_together" in value)
+        ):
+            print(f"__getattr__({name!r}) = {val!r}")
+        return val
+
+    def __setattr__(self, name, value):
+        if name == "options" and (
+            "unique_together" in value or "index_together" in value
+        ):
+            import inspect, textwrap
+
+            if (
+                "unique_together" in value
+                and not isinstance(value["unique_together"], set | tuple)
+                or "index_together" in value
+                and not isinstance(value["index_together"], set | tuple)
+            ):
+                print(f"__setattr__({name!r}, {value!r})")
+                for i in range(1, 4):
+                    calling_frame = inspect.stack(3)[i]
+                    code = textwrap.indent("".join(calling_frame.code_context), "    ")
+                    print(
+                        f"  {calling_frame.filename}:{calling_frame.lineno} in {calling_frame.function}\n{code}"
+                    )
+        return super().__setattr__(name, value)
+
     @cached_property
     def name_lower(self):
         return self.name.lower()

[Jacob Walls]

Left a suggestion for covering uncovered code changes.

[Jacob Walls]

Ready to go pending a flake8 fix.

[Jacob Walls <jacobtylerwalls@…>]

In 83622b82:

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]: ​https://github.com/django/django/commit/67dcea711e92025d0e8676b869b7ef15dbc6db73#diff-5dd147e9e978e645313dd99eab3a7bab1f1cb0a53e256843adb68aeed71e61dcR85-R87
[}2]: ​https://github.com/django/django/blob/b1ffa9a9d78b0c2c5ad6ed5a1d84e380d5cfd010/django/db/migrations/state.py#L842
[^{3]: ​https://github.com/django/django/blob/b1ffa9a9d78b0c2c5ad6ed5a1d84e380d5cfd010/django/db/migrations/state.py#L340-L345
[}4]: ​https://github.com/django/django/blob/b1ffa9a9d78b0c2c5ad6ed5a1d84e380d5cfd010/django/db/migrations/autodetector.py#L1757-L1771
[^{5]: ​https://github.com/django/django/blob/2351c1b12cc9cf82d642f769c774bc3ea0cc4006/django/core/management/commands/makemigrations.py#L215-L219
[}6]: ​https://github.com/django/django/blob/2351c1b12cc9cf82d642f769c774bc3ea0cc4006/django/core/management/commands/migrate.py#L329-L332