#34333 closed Bug (fixed)

Migrations tries to add constraint before adding a foreign key.

Reported by: Raphael Beekmann Owned by: Durval Carvalho
Component: Migrations Version: 4.1
Severity: Normal Keywords: migration, constraint, field
Cc: Durval Carvalho, David Wobrock Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Raphael Beekmann)

Hello,

I have a model, already created through previous migrations, and in a new migration I added a new field with an UniqueConstraint. The migrations script try to create the constraint first and then the new field, resulting an error :

django.core.exceptions.FieldDoesNotExist: NewModel has no field named 'category'

To reproduce the bug :

  1. Create a project with two models linked together with a One-to-Many relation and an unique constraint :
class Type(models.Model):
    name = models.CharField(max_length=10)


class Model(models.Model):
    name = models.CharField(max_length=10)
    type = models.ForeignKey(Type, on_delete=models.SET_NULL, null=True)
    date = models.DateField(auto_now=True)

    class Meta:
        constraints = (
            models.UniqueConstraint(fields=('date', 'type'), name='unique_type_for_date'),
        )
  1. Create a migration file with manage.py makemigrations
  1. Add a new model with another One-to-Many relation and unique constraint. The models looks like this :
class Type(models.Model):
    name = models.CharField(max_length=10)


class Category(models.Model):
    name = models.CharField(max_length=10)


class Model(models.Model):
    name = models.CharField(max_length=10)
    type = models.ForeignKey(Type, on_delete=models.SET_NULL, null=True)
    category = models.ForeignKey(Category, on_delete=models.SET_NULL, null=True)
    date = models.DateField(auto_now=True)

    class Meta:
        constraints = (
            models.UniqueConstraint(fields=('date', 'type'), name='unique_type_for_date'),
            models.UniqueConstraint(fields=('date', 'category'), name='unique_category_for_date'),
        )
  1. Create a new migration file. The order of the migration's steps are incorrect and the migration crash :
class Migration(migrations.Migration):

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

    operations = [
        migrations.CreateModel(
            name='Category',
            fields=[
                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('name', models.CharField(max_length=10)),
            ],
        ),
        migrations.AddConstraint(
            model_name='model',
            constraint=models.UniqueConstraint(fields=('date', 'category'), name='unique_category_for_date'),
        ),
        migrations.AddField(
            model_name='model',
            name='category',
            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='app.category'),
        ),
    ]

Change History (11)

comment:1 by Mariusz Felisiak, 23 months ago

Resolution: needsinfo
Status: newclosed

Thanks for the report, however I cannot reproduce this issue. For me, makemigrations generates operations in the correct order. Please reopen the ticket if you can debug your issue and provide a small project that reproduces it.

comment:2 by Raphael Beekmann, 23 months ago

Description: modified (diff)
Resolution: needsinfo
Status: closednew

comment:3 by Mariusz Felisiak, 23 months ago

Summary: Django migrations adds constraint before adding fieldMigrations tries to add constraint before adding a foreign key.
Triage Stage: UnreviewedAccepted

Thanks for the reproducible scenario.

comment:4 by Simon Charette, 23 months ago

For anyone interested in fixing this issue the bug lies in django.db.migrations.autodetector.MigrationAutodetector in each alteration and addition methods that relate to indexes and constraints and call add_operation without specifying dependencies.

If you look at how _generate_altered_foo_together is implement it does it right by building dependencies for foreign keys references in (index|unique)_together. The same needs to be done for indexes and constraints but since they also support expressions and conditions these needs to be considered as well.

Basically a set of fields references from .fields, .expressions, .include and .condition has to be extracted and for each Index and Constraint and _get_dependencies_for_foreign_key has to be called for each remote fields and combined to be passed as dependencies to the add_operation call.

Doing so for .fields and .include are trivial, for .expressions and .condition I would refer to Expression.flatten as it's used in UniqueConstraint.validate that might be a good reason to add Expression.field_refs method of simply make F.get_refs return {self.name} so Expression.get_refs can be used generically (for alias and field references).

All that to say this is #25551 back from the dead 7 years later with new APIs.

comment:5 by Sota Tabu, 23 months ago

Owner: changed from nobody to Sota Tabu
Status: newassigned

comment:6 by Durval Carvalho, 22 months ago

Owner: changed from Sota Tabu to Durval Carvalho

comment:7 by Durval Carvalho, 22 months ago

Cc: Durval Carvalho added
Needs tests: set

comment:8 by Simon Charette, 22 months ago

Has patch: set
Needs tests: unset

comment:9 by David Wobrock, 22 months ago

Cc: David Wobrock added
Patch needs improvement: set

For the tiny things to fix in the PR. 😉

comment:10 by Mariusz Felisiak, 22 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 22 months ago

Resolution: fixed
Status: assignedclosed

In 4b1bfea:

Fixed #34333 -- Fixed migration operations ordering when adding index/constraint on new foreign key.

Thanks Simon Charette and David Wobrock for reviews.

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