Opened 9 years ago
Last modified 5 weeks ago
#26739 assigned Bug
Backward operation for RemoveField does not allow a default value in case the field is not null.
Reported by: | Gagaro | Owned by: | Bhuvnesh |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | wkoot, Bhuvnesh | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Pull Requests: | 16890 build:success, | ||
Description ¶
If a field does not allow NULL and has no default, and this field is removed using RemoveField, the backward operation will break if the database is populated because the field will be created with no value.
Replication:
models.py
class TestModel(Model): field = CharField(max_length=255)
$ python manage.py makemigrations $ python manage.py migrate $ python manage.py shell >>> TestModel.objects.create(field='test')
models.py
class TestModel(Model): pass
$ python manage.py makemigrations $ python manage migrate $ python manage migrate app 0001
POC Patch made by apollo13 in IRC:
diff --git a/django/db/migrations/operations/fields.py b/django/db/migrations/operations/fields.py index 041f8a6..5604e36 100644 --- a/django/db/migrations/operations/fields.py +++ b/django/db/migrations/operations/fields.py @@ -122,6 +122,10 @@ class RemoveField(FieldOperation): Removes a field from a model. """ + def __init__(self, *args, **kwargs): + self.default = kwargs.pop('default', None) + super(RemoveField, self).__init__(*args, **kwargs) + def deconstruct(self): kwargs = { 'model_name': self.model_name, @@ -150,7 +154,12 @@ class RemoveField(FieldOperation): to_model = to_state.apps.get_model(app_label, self.model_name) if self.allow_migrate_model(schema_editor.connection.alias, to_model): from_model = from_state.apps.get_model(app_label, self.model_name) - schema_editor.add_field(from_model, to_model._meta.get_field(self.name)) + to_field = to_model._meta.get_field(self.name) + old_default = to_field.default + if self.default is not None: + to_field.default = self.default + schema_editor.add_field(from_model, to_field) + to_field.default = old_default def describe(self): return "Remove field %s from %s" % (self.name, self.model_name)
According to the ticket's flags, the next step(s) to move this issue forward are:
- To improve the patch as described in the pull request review comments or on this ticket, then uncheck "Patch needs improvement".
If creating a new pull request, include a link to the pull request in the ticket comment when making that update. The usual format is:
[https://github.com/django/django/pull/#### PR]
.
Change History (14)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 9 years ago
It would be great if we could also rely on the transient default of AddField
(preserve_default=False
) if it has been provided.
e.g. default should not be required in this case
operations = [ AddField('model', 'field', models.TextField(default='foo'), preserve_default=False), RemoveField('model', 'field'), ]
comment:3 by , 9 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 9 years ago
Patch needs improvement: | set |
---|
comment:5 by , 6 years ago
Cc: | added |
---|
comment:6 by , 21 months ago
Cc: | added |
---|
comment:7 by , 21 months ago
Owner: | changed from | to
---|
comment:9 by , 13 months ago
Patch needs improvement: | unset |
---|
comment:10 by , 11 months ago
Needs documentation: | set |
---|
comment:11 by , 10 months ago
Needs documentation: | unset |
---|
comment:12 by , 10 months ago
Patch needs improvement: | set |
---|
comment:13 by , 7 weeks ago
Patch needs improvement: | unset |
---|
comment:14 by , 5 weeks ago
Patch needs improvement: | set |
---|
The autodetector should be updated to ask for a new default, tests and docs missing.