[Django] #36878: Migration's ModelState has varying type for unique_together and index_together options causing autodetector crash

[Django]

#36878: Migration's ModelState has varying type for unique_together and
index_together options causing autodetector crash
-------------------------------------+-------------------------------------
Reporter: Markus | Owner: Markus Holtermann
Holtermann |
Type: | Status: assigned
Uncategorized |
Component: | Version: dev
Migrations |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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):

{{{#!python
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
([https://github.com/django/django/blob/b1ffa9a9d78b0c2c5ad6ed5a1d84e380d5cfd010/django/db/migrations/state.py#L841
see the ModelState.from_model() method]).

Unfortunately, there are situations inside the `ProjectState`'s mutation
functions
([https://github.com/django/django/blob/b1ffa9a9d78b0c2c5ad6ed5a1d84e380d5cfd010/django/db/migrations/state.py#L340-L345
e.g. rename_field()]) where the data type for
`model_state.options["unique_together"]` is changed to `list[list[str]]`:

{{{#!python
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]
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36878>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36878: Migration's ModelState has varying type for unique_together and
index_together options causing autodetector crash
-------------------------------------+-------------------------------------

Reporter: Markus Holtermann | Owner: Markus
| Holtermann
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* stage: Unreviewed => Accepted
* type: Uncategorized => Bug

--
Ticket URL: <https://code.djangoproject.com/ticket/36878#comment:1>

[Django]

#36878: Migration's ModelState has varying type for unique_together and
index_together options causing autodetector crash
-------------------------------------+-------------------------------------
Reporter: Markus Holtermann | Owner: Markus
| Holtermann
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Markus Holtermann):

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

{{{#!diff
diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py
index 057651b1df..7220673142 100644
--- a/django/db/migrations/state.py
+++ b/django/db/migrations/state.py
@@ -734,6 +734,32 @@ class StateApps(Apps):
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,7 +777,7 @@ class ModelState:
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,6 +809,37 @@ class ModelState:
"%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()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36878#comment:2>

[Django]

#36878: Migration's ModelState has varying type for unique_together and
index_together options causing autodetector crash
-------------------------------------+-------------------------------------
Reporter: Markus Holtermann | Owner: Markus
| Holtermann
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Markus Holtermann):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36878#comment:3>