Opened 23 months ago
Closed 22 months ago
#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 )
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 :
- 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'), )
- Create a migration file with
manage.py makemigrations
- 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'), )
- 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 , 23 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 23 months ago
Description: | modified (diff) |
---|---|
Resolution: | needsinfo |
Status: | closed → new |
comment:3 by , 23 months ago
Summary: | Django migrations adds constraint before adding field → Migrations tries to add constraint before adding a foreign key. |
---|---|
Triage Stage: | Unreviewed → Accepted |
Thanks for the reproducible scenario.
comment:4 by , 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 , 23 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 22 months ago
Owner: | changed from | to
---|
comment:7 by , 22 months ago
Cc: | added |
---|---|
Needs tests: | set |
comment:8 by , 22 months ago
Has patch: | set |
---|---|
Needs tests: | unset |
comment:9 by , 22 months ago
Cc: | added |
---|---|
Patch needs improvement: | set |
For the tiny things to fix in the PR. 😉
comment:10 by , 22 months ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
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.