Migration using CreateModel with unique_together followed by AlterUniqueTogether crashes

[Adam Johnson]

Full project to reproduce: ​https://github.com/adamchainz/django-unique-together-bug

When squashing migrations I ended up with a migration that added a model with unique_together, then altered the unique_together later:

migrations.CreateModel(
    name="Book",
    fields=[
        (
            "id",
            models.AutoField(
                auto_created=True,
                primary_key=True,
                serialize=False,
                verbose_name="ID",
            ),
        ),
        ("title", models.CharField(default="", max_length=100)),
    ],
    options={"unique_together": {("title",)},},
),
# ...
migrations.AlterUniqueTogether(
    name="book", unique_together={("title", "author")},
),

This can crash when using sqlmigrate or migrate (see repo) with:

  File "/.../django/django/db/backends/base/schema.py", line 380, in alter_unique_together
    self._delete_composed_index(model, fields, {'unique': True}, self.sql_delete_unique)
  File "/.../django/django/db/backends/base/schema.py", line 414, in _delete_composed_index
    ", ".join(columns),
ValueError: Found wrong number (0) of constraints for testapp_book(title)

This is because _delete_composed_index from AlterUniqueTogehter tries to find the existing constraint name from the database.. But it hasn't been created yet, so it could never be found - its SQL is sitting in deferred_sql.

[Simon Charette]

Which version of Django did you use to squash the migrations? A [CreateModel, AlterUniqueTogether] sequence should get optimized into [CreateModel]

​https://github.com/django/django/blob/e65fea9292ffdeb9bb76062f6cb2a5ff514ae969/django/db/migrations/operations/models.py#L145-L154

[Adam Johnson]

2.2, 3.0, master

It isn't squashed in this case because the unique_together includes a FK.

​https://github.com/adamchainz/django-unique-together-bug/blob/master/testproj/testapp/migrations/0001_squashed_0002_add_author.py

# Generated by Django 2.1.15 on 2020-02-28 06:56

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

    replaces = [("testapp", "0001_initial"), ("testapp", "0002_add_author")]

    initial = True

    dependencies = []

    operations = [
        migrations.CreateModel(
            name="Book",
            fields=[
                (
                    "id",
                    models.AutoField(
                        auto_created=True,
                        primary_key=True,
                        serialize=False,
                        verbose_name="ID",
                    ),
                ),
                ("title", models.CharField(default="", max_length=100)),
            ],
            options={"unique_together": {("title",)},},
        ),
        migrations.CreateModel(
            name="Author",
            fields=[
                (
                    "id",
                    models.AutoField(
                        auto_created=True,
                        primary_key=True,
                        serialize=False,
                        verbose_name="ID",
                    ),
                ),
            ],
        ),
        migrations.AddField(
            model_name="book",
            name="author",
            field=models.ForeignKey(
                null=True,
                on_delete=django.db.models.deletion.DO_NOTHING,
                to="testapp.Author",
            ),
        ),
        migrations.AlterUniqueTogether(
            name="book", unique_together={("title", "author")},
        ),
    ]

It's also possible to hand write such a migration.

[Simon Charette]

We'll likely have to use a specialized ddl_statement.Statement subclass when deferring constraints and indices and have _delete_composed_index consider them by looping through dereferred_sql before raising that exception.

That would have the benefit of also avoiding creating constraints meant to be immediately dropped.

[Adam Johnson]

An alternative would be to translate unique_together = [['a']] into an extra constraint in constraints and then it can be treated by migrations as AddConstraint / RemoveConstraint` operations. Though that might mean more work.

[Simon Charette]

That approach would also not account for constraint name instabilities that shipped over the past years in various Django versions.

[Adam Johnson]

Ah I was not aware the algorithm has been unstable.