Changing an IntegerField to a ForeignKey generates incorrectly ordered migration operations if the field is in unique_together

[Ed Morley]

Replacing an integer field with a foreign key of the same name, results in an OperationalError when creating/applying the migration.

This affects Django master + v1.11.10, and both the SQLite and MySQL backends (others not tested).

STR:

1. Git clone ​https://github.com/edmorley/django-migration-int-to-fk-testcase
2. pip install https://github.com/django/django/archive/master.zip
3. ./manage.py migrate
4. cp testapp/models_new.py testapp/models.py
5. ./manage.py makemigrations --name broken_migration
6. ./manage.py migrate

Expected:

New migration is created/applied successfully, which converts from the ​original model to the ​new model.

Actual:

The new 0002_broken_migration.py migration incorrectly lists the AddField operation before the RemoveField operation...

    operations = [
        migrations.AddField(
            model_name='bar',
            name='foo',
            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='testapp.Foo'),
        ),
        migrations.RemoveField(
            model_name='bar',
            name='foo_id',
        ),
        migrations.AlterUniqueTogether(
            name='bar',
            unique_together={('name', 'foo')},
        ),
    ]

Which results in an exception at step 6...

$ ./manage.py migrate
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, sessions, testapp
Running migrations:
  Applying testapp.0002_broken_migration...Traceback (most recent call last):
  File ".../site-packages/django/db/backends/utils.py", line 83, in _execute
    return self.cursor.execute(sql)
  File ".../site-packages/django/db/backends/sqlite3/base.py", line 290, in execute
    return Database.Cursor.execute(self, query)
sqlite3.OperationalError: duplicate column name: foo_id

Additional notes:

• Without the unique_together on model Bar, the bug does not occur.
• This affects both the SQLite backend and the MySQL backend (others not tested).
• At time of testing, django master was at revision 6d794fb76212bb8a62fe2cd97cff173054e1c626.
• The above was using Python 3.6.2.

[Tim Graham]

I didn't attempt to reproduce but the report looks credible.

[Jeff]

I was able to reproduce and am starting work.

[Jeff]

It looks like the example migrations provided are exposing two separate issues.

1) improperly ordered AddField/RemoveField/AlterUniqueTogether. Below is the initial migration

Original Models:

    class Foo(models.Model):
        pass


    class Bar(models.Model):
        name = models.CharField(max_length=50)
        foo_id = models.PositiveIntegerField()

        class Meta:
            unique_together = ('name', 'foo_id')

Updated Models:

    class Foo(models.Model):
        pass


    class Bar(models.Model):
        name = models.CharField(max_length=
        baz = models.ForeignKey(Foo, models.CASCADE, null=True)

        class Meta:
            unique_together = ('name', 'baz')

This produces the problematic migration:

    class Migration(migrations.Migration):

        dependencies = [
            ('temp', '0001_initial'),
        ]

        operations = [
            migrations.AddField(
                model_name='bar',
                name='baz',
                field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='temp.Foo'),
            ),
            migrations.RemoveField(
                model_name='bar',
                name='foo_id',
            ),
            migrations.AlterUniqueTogether(
                name='bar',
                unique_together={('name', 'baz')},
            ),
        ]

Instead of this functional migration:

    class Migration(migrations.Migration):

        dependencies = [
            ('temp', '0001_initial'),
        ]

        operations = [
            migrations.AddField(
                model_name='bar',
                name='baz',
                field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='temp.Foo'),
            ),
            migrations.AlterUniqueTogether(
                name='bar',
                unique_together={('name', 'baz')},
            ),
            migrations.RemoveField(
                model_name='bar',
                name='foo_id',
            ),
        ]

and 2) problematic behavior around the clashing of names like foo with foo_id.

I believe the given examples somewhat conflated these two separate issues, while I believe 1) is a bug and should be fixed, the I believe 2) is working as intended.
Please advise if I am incorrect on this assumption.

[Jeff]

I have tracked down the issue to the migration optimization. Debugging now to determine how to ensure a field cannot be removed while it is a part of a unique_together constraint.

[Ramiro Morales]

Looks suspiciously similar or at least related to #28862.

[Jeff]

Thanks for the tip, I do believe it to be the same issue.

[Jeff]

@TimGraham I believe this can be closed as a duplicate of #28862

[Ramiro Morales]

Duplicate of #28862