Opened 9 years ago
Last modified 8 months ago
#26223 new Bug
Squashing migrations with preserve_default=False keeps the default
Reported by: | Bartek Wójcicki | Owned by: | |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Markus Holtermann, InvalidInterrupt, Ülgen Sarıkavak | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
I have a migration with AlterField with default value provided and preserve_default=False. When I squash it, I got a CreateModel operation with default value for this field set.
Steps to reproduce:
- Create a model, make migrations.
class FooBar(models.Model): pass
- Add field without default, make migrations, provide a one-off default.
class FooBar(models.Model): foo = models.TextField()
- Squash migrations.
Squashed migration keeps the default in CreateModel operation.
operations = [ migrations.CreateModel( name='FooBar', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('foo', models.TextField(default='bar')), ], ), ]
Running makemigrations again creates a migration with AlterField, removing the default.
operations = [ migrations.AlterField( model_name='foobar', name='foo', field=models.TextField(), ), ]
Change History (13)
comment:1 by , 9 years ago
Description: | modified (diff) |
---|
comment:2 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 8 years ago
Version: | 1.8 → master |
---|
I'm also not sure about about the real_field
's naming but that sounds like the right approach to me.
comment:6 by , 8 years ago
Here's an alternative approach.
AddField.field
could be changed to mean the actual field that goes into model state. The transient database default value would be a separate attribute on the operation.
This would make code a bit cleaner because default
handling would only occur in database_forwards
(where it already occurs).
For backwards compatibility, constructor would still accept preserve_default
and modify the field if needed.
makemigrations
and squashmigrations
would emit the operation with the new signature.
If preserve_default
is deprecated, we can suggest users to modify existing migrations by hand or simply squash them.
Same goes for AlterField
.
If this approach is worth considering, I could make another PR to see how it looks.
comment:7 by , 8 years ago
I found another faulty optimization for AddField
+AlterField
:
>>> optimize([ ... migrations.AddField("Foo", "value", models.IntegerField(default=42), preserve_default=False), ... migrations.AlterField("Foo", "value", models.IntegerField("Value")), ... ]) [ migrations.AddField( model_name='Foo', name='value', field=models.IntegerField(verbose_name='Value'), ), ]
PR updated to deal with it.
comment:8 by , 8 years ago
Cc: | added |
---|
Markus, any thoughts about the approach in the current PR vs. the alternative suggested in comment:6?
comment:9 by , 8 years ago
I have a WIP patch for the approach from comment:6, if anyone's interested: https://github.com/vytisb/django/pull/1
comment:10 by , 7 years ago
Patch needs improvement: | set |
---|
comment:11 by , 4 years ago
Cc: | added |
---|
comment:12 by , 20 months ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:13 by , 8 months ago
Cc: | added |
---|
I have analyzed the migration optimizer code and identified 5 cases where
preserve_default
is not handled correctly.For the cases involving
CreateModel
, I intend to add a new propertyreal_field
(suggestions for a better name are welcome) toAddField
/AlterField
which would return a possibly modified field after takingpreserve_default
into account. It would be the same field that these operations put into state. ThenCreateModel
could use the new property in its optimization.For the other cases, proper passing of
preserve_default
should be sufficient.