[Django] #23048: RemoveField irreversible in certain cases
Django
--------------------------------+----------------------
Reporter: harrislapiroff | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+----------------------
RemoveField is an irreversible operation if a field's original state
requires it be non-null.
To reproduce:
* Write a model with a field with `null=False`
* Make and run initial migrations.
* Create and save some instances of your model.
* Remove the field.
* Make and run the migration.
* Attempt to migrate back to the initial migration (i.e., `python
manage.py migrate myapp 0001`)
You'll get `django.db.utils.IntegrityError: myapp_mymodel__new.field may
not be NULL`
This behavior is documented (https://docs.djangoproject.com/en/dev/ref
/migration-operations/#django.db.migrations.operations.RemoveField), but I
believe there should be a way to provide a default value for the field
when unapplying the migration.
--
Ticket URL: <https://code.djangoproject.com/ticket/23048>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: harrislapiroff | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7-rc-1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ihulub):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I believe there is a way to provide a default value for the field.
Have you tried passing the "default=xyz" parameter to the model field?
Example:
name = model.CharField(null=False, default="noname")
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:1>
Django
--------------------------------+------------------------------------
Reporter: harrislapiroff | Owner: nobody
Type: New feature | Status: new
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* type: Bug => New feature
* stage: Unreviewed => Accepted
Comment:
I don't think adding a `default` on the model field would make any
difference. Anyway, it seems like a reasonable feature request although
I'm not sure how feasible it will be; tentatively accepting, although
someone else can feel free to close with a rationale of why we can't do
it.
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:2>
Django
--------------------------------+------------------------------------
Type: New feature | Status: closed
Component: Migrations | Version: 1.7-rc-1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by andrewgodwin):
* status: new => closed
* resolution: => wontfix
Comment:
Adding a default on the model field or on the field in the migration
should solve this, as it'll be able to populate the new field with that
when it makes it. The only other option is to prompt people for a default
every time they *remove* a not null field, which really confused people in
South, so let's not do that.
Closing as WONTFIX, as the solution is a simple edit to the migration.
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:3>
Django
Reporter: Harris Lapiroff | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Datamance):
Why was this closed as wontfix?
The provided "solutions" from the last couple of comments don't address
the fact that developers often take over projects that don't belong to
them initially; I can't go back in time and ask the developer to put
defaults on a NOT NULL field. Furthermore, solutions like inserting
AlterField in the migration itself clearly don't help when the migration
breaks precisely at RemoveField.
There is a completely reasonable fix to this and it's to add something
like a "reverse override" object in the migrations.RemoveField constructor
that allows you to change the field-adding behavior of the reversed query.
Something like
{{{
#!div style="font-size: 80%"
Code highlighting:
{{{#!python
migrations.RemoveField(model_name="child", name="age_at_birth",
reversal_options={"null": True}),
}}}
}}}
Is a clean enough API and provides the necessary state to go backwards and
forwards cleanly.
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:4>
Django
---------------------------------+------------------------------------
Reporter: Harris Lapiroff | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by djsmedes):
It seems like the clearest interface and most general solution would be to
allow manually specifying the reverse operation as an entire object. With
`preserve_default=False` you can then set a default on a field that was
nullable. For example:
{{{
# migration 0003
...
operations = [
migrations.AddField(
model_name='invitation',
name='first_name',
field=models.TextField(default=''),
preserve_default=False,
),
]
...
# migration 0004
...
operations = [
migrations.RemoveField(
model_name='invitation',
name='first_name',
reverse_operation=migrations.AddField( # <-- this is what I am
proposing
...
field=models.TextField(default=''),
preserve_default=False,
...
),
),
...
}}}
This could be applied to `AlterField` as well I would think.
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:5>
Django
---------------------------------+------------------------------------
Reporter: Harris Lapiroff | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
@Rico
> I can't go back in time and ask the developer to put defaults on a NOT
NULL field.
You simply have to edit the migration that added the field and set a
`default` and `preserve_default=False` to the `AddField` or `CreateModel`
operation.
@Daniel
If `default` is defined on the field like in your example (`migration
0003`) the migration framework will already knows that it must recreate
the column with the provided default on the `RemoveField` reversal. I'm
not sure the added complexity of `reverse_operation` is worth it.
Maybe we could raise a warning/notice to users when generating a migration
with a `RemoveField` for a field that doesn't define a default to suggest
editing the migration that introduced the field to add a default?
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:6>
Django
---------------------------------+------------------------------------
Reporter: Harris Lapiroff | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by rm_):
Found this debugging a similar issue. My problem comes from having some
migrations tests that test a specific migrations for a field that I'm
trying to remove. So in my migrations history I have: addfield -> make
nullable -> remove field
I've tried to add a default as suggested above:
- field=models.ForeignKey(blank=True, null=True,
on_delete=django.db.models.deletion.CASCADE,
to='translatable_messages.Language'),
+ field=models.ForeignKey(blank=True, null=True,
on_delete=django.db.models.deletion.CASCADE,
to='translatable_messages.Language', default=1),
+ preserve_default=False
but instead of postgres warning about the field having null values when
it's not supposed to i get this:
django.db.utils.OperationalError: cannot ALTER TABLE
"translatable_messages_translatablemessage" because it has pending trigger
events
I'm tempted to remove the tests on the migrations and call it a day but
maybe there's something else I can do?
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:7>
Django
---------------------------------+------------------------------------
Reporter: Harris Lapiroff | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by rm_):
* cc: rm_ (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/23048#comment:8>