Opened 8 years ago

Last modified 7 days 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

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)

Change History (12)

comment:1 by Florian Apolloner, 8 years ago

Triage Stage: UnreviewedAccepted

The autodetector should be updated to ask for a new default, tests and docs missing.

comment:2 by Simon Charette, 8 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 Markus Holtermann, 8 years ago

Has patch: set
Owner: changed from nobody to Markus Holtermann
Status: newassigned

comment:4 by Simon Charette, 8 years ago

Patch needs improvement: set

comment:5 by wkoot, 5 years ago

Cc: wkoot added

comment:6 by Mariusz Felisiak, 12 months ago

Cc: Bhuvnesh added

comment:7 by Bhuvnesh, 12 months ago

Owner: changed from Markus Holtermann to Bhuvnesh

comment:9 by Sarah Boyce, 3 months ago

Patch needs improvement: unset

comment:10 by Sarah Boyce, 4 weeks ago

Needs documentation: set

comment:11 by Sarah Boyce, 2 weeks ago

Needs documentation: unset

comment:12 by Sarah Boyce, 7 days ago

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top