Opened 3 years ago
Closed 3 years 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 , 3 years ago
| Resolution: | → needsinfo |
|---|---|
| Status: | new → closed |
comment:2 by , 3 years ago
| Description: | modified (diff) |
|---|---|
| Resolution: | needsinfo |
| Status: | closed → new |
comment:3 by , 3 years 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 , 3 years 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 , 3 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 3 years ago
| Owner: | changed from to |
|---|
comment:7 by , 3 years ago
| Cc: | added |
|---|---|
| Needs tests: | set |
comment:8 by , 3 years ago
| Has patch: | set |
|---|---|
| Needs tests: | unset |
comment:9 by , 3 years ago
| Cc: | added |
|---|---|
| Patch needs improvement: | set |
For the tiny things to fix in the PR. 😉
comment:10 by , 3 years ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
Thanks for the report, however I cannot reproduce this issue. For me,
makemigrationsgenerates operations in the correct order. Please reopen the ticket if you can debug your issue and provide a small project that reproduces it.