[Django] #25105: Migrating multiple CharFields to null=False breaks on PostgreSQL
Django
----------------------------+--------------------
Reporter: danielr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
Given a model containing multiple CharFields defined as {{{null=True}}},
and at least one row with null data in two or more of those fields, then
change the CharFields to null=False and create a migration with a
temporary default of the empty string.
Running this migration will attempt, for each field, to set the null
values to the empty string, then migrate the field to NOT NULL. However,
the second field will fail with the error {{{psycopg2.OperationalError:
cannot ALTER TABLE "fundraising_donation" because it has pending trigger
events}}}.
Clearly this is the same issue warned against in the
[https://docs.djangoproject.com/en/1.8/ref/migration-operations/#runpython
migration documentation] as caused by mixing RunPython and schema changes
in the same migration; however, here it is a fully automatically-generated
migration which causes the error.
Some possible solutions:
* Do nothing but mention this edge case in the documentation;
* Detect and warn against converting multiple null=False fields in one
migration;
* Detect and deal with the problem in some way, perhaps by committing the
transaction after each field.
--
Ticket URL: <https://code.djangoproject.com/ticket/25105>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: danielr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
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 timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Is modifying multiple fields necessary to reproduce? I thought one field
would be enough as long as the `UPDATE` query modifies some rows, but that
was just my intuition.
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:1>
Django
Reporter: danielr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Zanuda999):
I created model like
from django.db import models
class Test(models.Model):
text1 = models.CharField(null=True, blank=True, max_length=255)
text2 = models.CharField(null=True, blank=True, max_length=255)
then i created initial migration, migrated, created 2 objects with null
values in text1 and text2, changed all fields to null=False, blank=False,
migrated and all is ok. i have installed postgresql 9.4.2
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:2>
Django
Reporter: danielr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by danielr):
On further investigation, in turns out that a) you do need at least two
fields being migrated, and b) you also need a {{{ForeignKey}}}, which is
why Zanuda999 couldn't replicate it. So, given these models:
{{{
class Model1(models.Model):
field1 = models.CharField(max_length=20)
class Model2(models.Model):
field1 = models.CharField(max_length=20, blank=True, null=True)
field2 = models.CharField(max_length=20, blank=True, null=True)
model1 = models.ForeignKey(Model1)
}}}
Create and run the migrations, then in the shell:
{{{
>>> m=Model1.objects.create(field1='foo')
>>> Model2.objects.create(model1=m)
}}}
Now, delete the {{{null=True}}} from Model2.field1 and field2, and create
migrations again, using a one-off default of {{{''}}} for both fields.
Running migrations now produces this error:
{{{
Running migrations:
Rendering model states... DONE
Applying null_to_blank.0002_auto_20150711_1244...Traceback (most recent
call last):
File "./manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File
"/Users/danielroseman/Projects/django/django/core/management/__init__.py",
line 331, in execute_from_command_line
utility.execute()
File
"/Users/danielroseman/Projects/django/django/core/management/__init__.py",
line 323, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File
"/Users/danielroseman/Projects/django/django/core/management/base.py",
line 350, in run_from_argv
self.execute(*args, **cmd_options)
File
"/Users/danielroseman/Projects/django/django/core/management/base.py",
line 401, in execute
output = self.handle(*args, **options)
File
"/Users/danielroseman/Projects/django/django/core/management/commands/migrate.py",
line 195, in handle
executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
File
"/Users/danielroseman/Projects/django/django/db/migrations/executor.py",
line 110, in migrate
self.apply_migration(states[migration], migration, fake=fake,
fake_initial=fake_initial)
File
"/Users/danielroseman/Projects/django/django/db/migrations/executor.py",
line 148, in apply_migration
state = migration.apply(state, schema_editor)
File
"/Users/danielroseman/Projects/django/django/db/migrations/migration.py",
line 116, in apply
operation.database_forwards(self.app_label, schema_editor, old_state,
project_state)
File
"/Users/danielroseman/Projects/django/django/db/migrations/operations/fields.py",
line 201, in database_forwards
schema_editor.alter_field(from_model, from_field, to_field)
File
"/Users/danielroseman/Projects/django/django/db/backends/base/schema.py",
line 482, in alter_field
old_db_params, new_db_params, strict)
File
"/Users/danielroseman/Projects/django/django/db/backends/base/schema.py",
line 654, in _alter_field
params,
File
"/Users/danielroseman/Projects/django/django/db/backends/base/schema.py",
line 110, in execute
cursor.execute(sql, params)
File "/Users/danielroseman/Projects/django/django/db/backends/utils.py",
line 79, in execute
return super(CursorDebugWrapper, self).execute(sql, params)
File "/Users/danielroseman/Projects/django/django/db/backends/utils.py",
line 64, in execute
return self.cursor.execute(sql, params)
File "/Users/danielroseman/Projects/django/django/db/utils.py", line 95,
in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/Users/danielroseman/Projects/django/django/db/backends/utils.py",
line 64, in execute
return self.cursor.execute(sql, params)
django.db.utils.OperationalError: cannot ALTER TABLE
"null_to_blank_model2" because it has pending trigger events
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:3>
Django
Reporter: danielr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by spookylukey):
I've also seen this in production with a '''single''' `AlterField`
migration. I saw this with some Postgres versions (9.1.16) and not others
(9.3.6). I've been able to fix it by manually running `UPDATE foo SET bar
= '' WHERE bar IS NULL;` and re-running the migration.
With other tables in the same DB, simply splitting into one migration per
field is enough to fix it. I haven't been able to investigate further.
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:4>
Django
Reporter: danielr | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by rafadev):
Seems to be happening also at least with PositiveIntegerField, even
specifying a default when creating the migration.
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:5>
Django
Reporter: Daniel Roseman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Michał Łazowik):
* cc: Michał Łazowik (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:6>
Django
Reporter: Daniel Roseman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
It looks like using an approach similar to #25492
(cd7efa20338cb6f3ede4780e00590c0a6dd48ca2) could work here.
We should be checking all deferred constraints (Django only creates
deferred constraint for foreign keys AFAIK) on tables we perform `UPDATE`
on.
An easy solution would be to append `';SET CONSTRAINTS ALL IMMEDIATE;SET
CONSTRAINTS ALL DEFERRED'` to Postgres' schema editor
`sql_update_with_default` attribute to force a constraint check. Postgres
should be smart enough to only check the required tables, something I
noticed when implementing support for the `table_names` parameter of the
`check_constraint` function.
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:7>
Django
Reporter: Daniel Roseman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Giovanni Totaro - aka Vanni):
[https://code.djangoproject.com/ticket/25105#comment:4 The UPDATE work-
around in comment #4] worked for me too. Thanks Luke Plant.
(Postgres 9.5, Ubuntu 16.04.3 x86_64, Python 3.5.2, Django 1.11)
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:8>
Django
Reporter: Daniel Roseman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Oskar Haller):
Bug still exists in Django 2.2, PostgreSQL 11
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:9>
Django
Reporter: Daniel Roseman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Florian Demmer):
* cc: Florian Demmer (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:10>
Django
Reporter: Daniel Roseman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Stian Jensen):
* version: 1.8 => 3.2
Comment:
Is this a wontfix, or just forgotten? Still exists on Django 3.2.
I'm aware manual workarounds exist, but you need to be made aware of those
by finding this issue thread after trying to apply the migration on real
data.
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:11>
Django
Reporter: Daniel Roseman | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Currently (still) waiting for somebody to propose a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:12>
Django
Reporter: Daniel Roseman | Owner: David Wobrock
Type: Bug | Status: assigned
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by David Wobrock):
* cc: David Wobrock (added)
* owner: nobody => David Wobrock
* has_patch: 0 => 1
* status: new => assigned
Comment:
Hi there,
Here is a [https://github.com/django/django/pull/15650 PR] that tries to
tackle the issue.
Let me know what you think of the approach :)
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:13>
Django
Reporter: Daniel Roseman | Owner: David
| Wobrock
Type: Bug | Status: assigned
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:14>
Django
Reporter: Daniel Roseman | Owner: David
| Wobrock
Component: Migrations | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8f04473af1da1ed4e8a07e89205da4e70d69b586" 8f04473a]:
{{{
#!CommitTicketReference repository=""
revision="8f04473af1da1ed4e8a07e89205da4e70d69b586"
Fixed #25105 -- Checked deferred constraints before updating rows on
PostgreSQL.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25105#comment:15>