[Django] #26739: Backward operation for RemoveField does not allow a default value in case the field is not null.
Django
the field is not null.
----------------------------+--------------------
Reporter: Gagaro | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
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)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26739>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
the field is not null.
Reporter: Gagaro | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by apollo13):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
The autodetector should be updated to ask for a new default, tests and
docs missing.
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:1>
Django
the field is not null.
Reporter: Gagaro | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
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
{{{#!python
operations = [
AddField('model', 'field', models.TextField(default='foo'),
preserve_default=False),
RemoveField('model', 'field'),
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:2>
Django
the field is not null.
Reporter: Gagaro | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* status: new => assigned
* owner: nobody => MarkusH
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/6870
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:3>
Django
the field is not null.
Reporter: Gagaro | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:4>
Django
the field is not null.
Reporter: Gagaro | Owner: Markus Holtermann
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by wkoot):
* cc: wkoot (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:5>
Django
the field is not null.
Reporter: Gagaro | Owner: Markus Holtermann
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Mariusz Felisiak):
* cc: Bhuvnesh (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:6>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Bhuvnesh):
* owner: Markus Holtermann => Bhuvnesh
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:7>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Bhuvnesh):
[https://github.com/django/django/pull/16890 WIP PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:8>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sarah Boyce):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:9>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by Sarah Boyce):
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:10>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by Sarah Boyce):
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:11>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by Sarah Boyce):
--
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:12>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by Jacob Walls):
* needs_better_patch: 1 => 0
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:13>
Django
the field is not null.
Reporter: Gagaro | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by Sarah Boyce):
* needs_better_patch: 0 => 1
Ticket URL: <https://code.djangoproject.com/ticket/26739#comment:14>