Opened 16 months ago

Last modified 15 months 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: Markus Holtermann
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: 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 (4)

comment:1 Changed 16 months ago by Florian Apolloner

Triage Stage: UnreviewedAccepted

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

comment:2 Changed 16 months ago by Simon Charette

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 Changed 15 months ago by Markus Holtermann

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

comment:4 Changed 15 months ago by Simon Charette

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