#31788: Models migration with change field foreign to many and deleting unique
together.
----------------------------+-----------------------------------------
Reporter: budzichd | Owner: David Wobrock
Type: Bug | Status: assigned
Component: Migrations | Version: 3.0
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 David Wobrock):
* cc: David Wobrock (added)
* owner: nobody => David Wobrock
* has_patch: 0 => 1
* status: new => assigned
Comment:
Quite a funny bug actually in the autodetector and how we sort migration
with respect to their dependencies.
I can't exactly reproduce the described output, but something similar.
Here is a little dive into the issue.
When changing a field from FK to M2M, the autodetector goes within
`django.db.migrations.autodetector.MigrationAutodetector.generate_altered_fields`
and creates a `RemoveField`+`AddField`, which is fine and works.
But we also generate a `AlterUniqueTogether`...
Since we call `generate_removed_altered_unique_together` before
`generate_altered_fields`, one would expect it to work as expected.
And at first, we have the right operation order: `[AlterUniqueTogether,
RemoveField, AddField]`.
However, when sorting the operations with respect to their dependencies,
we trigger the dependency that `RemoveField` should be after
`AlterUniqueTogether`.
Even if this condition is already satisfied, the `stable_topological_sort`
changes the order and returns `[AlterUniqueTogether, AddField,
RemoveField]`. The dependency is still satisfied, but our Remove/Add
became an Add/Remove, which cannot work.
In `tests/utils_tests/test_topological_sort.py`
{{{
def test_preserve_satisfied_order(self):
dependency_graph = {1: set(), 2: {1}, 3: set()}
self.assertEqual(
stable_topological_sort([1, 2, 3], dependency_graph),
[1, 2, 3],
)
}}}
return
{{{
======================================================================
FAIL: test_preserve_satisfied_order
(utils_tests.test_topological_sort.TopologicalSortTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/david/django/tests/utils_tests/test_topological_sort.py",
line 29, in test_preserve_satisfied_order
self.assertEqual(
AssertionError: Lists differ: [1, 3, 2] != [1, 2, 3]
First differing element 1:
3
2
- [1, 3, 2]
+ [1, 2, 3]
}}}
on my env (and since we rely on sets, I suspect that's why I don't have
the same result as described in the ticket. I think that's why Simon is
seeing a `[AddField, AlterUniqueTogether, RemoveField]`, which also
satisfies the dependencies).
(on top of that, the autodetector then optimizes the operations, and the
`AddField` followed by `RemoveField` cancel each other out, and we end up
with a single `[AlterUniqueTogether]`).
Two possible approaches to fix this:
1. Making the `AddField` depend on `RemoveField`, so that we force their
order
2. Changing the topological sort, so that it preserves the input order as
much as possible
Since the second might imply a large set of changes, so I tried
implementing the first one in this
[https://github.com/django/django/pull/15738 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/31788#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018105b881e1-04663909-0f5e-46fa-b13b-10cbc5d7779c-000000%40eu-central-1.amazonses.com.
