Opened 21 months ago

Last modified 5 weeks ago

#26180 assigned Bug

Altering unique_together still sometimes missing deleted fields

Reported by: Julian Andrews Owned by: Akshesh Doshi
Component: Migrations Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (1)

ticket26180-temp-fix.diff (2.2 KB) - added by Akshesh Doshi 15 months ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 21 months ago by Tim Graham

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 Changed 15 months ago by Akshesh Doshi

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 `_sort_migrations` method corrects the order of these operations to bring AlterUniqueTogether before RemoveField. But the `_optimize_migrations` method again reverses this order in self.migrations (which is finally what is returned by the _detect_changes method) with no optimization as such.

This change is somewhat related to https://github.com/django/django/commit/e470f311d654267ec86f9a6325ec500345b9dff2

Last edited 15 months ago by Akshesh Doshi (previous) (diff)

Changed 15 months ago by Akshesh Doshi

Attachment: ticket26180-temp-fix.diff added

comment:3 Changed 15 months ago by Akshesh Doshi

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.

comment:4 Changed 15 months ago by Akshesh Doshi

Has patch: set
Owner: changed from nobody to Akshesh Doshi
Status: newassigned

comment:5 Changed 15 months ago by Tim Graham

Has patch: unset

Unchecking "Has patch" due to "not the right approach" comment on the PR.

comment:6 Changed 15 months ago by Akshesh Doshi

Has patch: set
Needs documentation: set
Needs tests: set

An initial implementation of the suggested approach - https://github.com/django/django/pull/7038

comment:7 Changed 7 months ago by Tim Graham

I closed #27933, #28084, and #28366 as duplicates.

Last edited 4 months ago by Tim Graham (previous) (diff)

comment:8 Changed 5 weeks ago by Cliff Dyer

This issue just bit me on django 1.11.4, and required a couple hours of debugging to sort out.

Note: See TracTickets for help on using tickets.
Back to Top