[Django] #26180: Altering unique_together still sometimes missing deleted fields
Django
-------------------------------+--------------------
Reporter: julianandrews | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
This is likely a missed edge case from #23614.
Steps to reproduce:
1. Create an app `spam`
2. Create a model `Spam`
{{{
class Spam(models.Model):
a = models.CharField(max_length=255)
b = models.CharField(max_length=255)
c = models.CharField(max_length=255)
class Meta:
unique_together = (
('a', 'b'),
('b', 'c'),
)
}}}
3. Make migrations
4. Delete field `Spam.c`
5. Delete the second `unique_together` constraint (`('b', 'c')`)
6. Make migrations
7. Run migrations
Output:
{{{
django.core.exceptions.FieldDoesNotExist: Spam has no field named u'c'
}}}
Note that the bug doesn't occur if no `unique togther` constraints remain.
Manually reversing the order of the operations in the migration (to remove
the index first) produces a functional migration.
--
Ticket URL: <https://code.djangoproject.com/ticket/26180>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: julianandrews | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
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 timgraham):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* type: Uncategorized => Bug
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:1>
Django
Reporter: julianandrews | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akki):
This is happening because the `RemoveField` operation is called before
calling the `AlterUniqueTogether` operation, resulting in the dropping of
the field before the index.
In the `autodetector.py`, although the `RemoveField` operations are
created before the `AlterUniqueTogether` operations but the
[https://github.com/django/django/blob/20d39325ca1da57a709f3ba38299dc7b0fc4bdfb/django/db/migrations/autodetector.py#L188
`_sort_migrations` method] corrects the order of these operations to bring
`AlterUniqueTogether` before `RemoveField`. But the
[https://github.com/django/django/blob/20d39325ca1da57a709f3ba38299dc7b0fc4bdfb/django/db/migrations/autodetector.py#L190
`_optimize_migrations` method] again reverses this order in
`self.migrations` (which is finally what gets returned by the
`_detect_changes` method.
This change is related to
https://github.com/django/django/commit/e470f311d654267ec86f9a6325ec500345b9dff2
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:2>
Django
Reporter: julianandrews | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akki):
* Attachment "ticket26180-temp-fix.diff" added.
Django
Reporter: julianandrews | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akki):
The patch temporarily fixes the issue (attaching for anyone who gets stuck
due to this bug) but some other edge cases would still get left. The
actual fix would involve some changes in `migrations.optimizer` or the
`reduce` methods of `AlterUniqueTogether` or `RemoveField` operations
AFAICS.
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:3>
Django
Reporter: julianandrews | Owner: akki
Type: Bug | Status: assigned
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akki):
* owner: nobody => akki
* status: new => assigned
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/6926
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:4>
Django
Reporter: julianandrews | Owner: akki
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 1 => 0
Comment:
Unchecking "Has patch" due to "not the right approach" comment on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:5>
Django
Reporter: julianandrews | Owner: akki
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akki):
* needs_docs: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
An initial implementation of the suggested approach -
https://github.com/django/django/pull/7038
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:6>
Django
Reporter: Julian Andrews | Owner: Akshesh Doshi
Type: Bug | Status: assigned
Component: Migrations | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I closed #27933 as a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:7>
Django
Reporter: Julian Andrews | Owner: Akshesh Doshi
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Cliff Dyer):
This issue just bit me on django 1.11.4, and required a couple hours of
debugging to sort out.
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:8>
Django
Reporter: Julian Andrews | Owner: Akshesh Doshi
Component: Migrations | Version: 1.9
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* status: assigned => closed
* resolution: => duplicate
Comment:
Pretty sure this is a duplicate of #28862 as well. I'll close this one as
since the other one has more context.
--
Ticket URL: <https://code.djangoproject.com/ticket/26180#comment:9>