Opened 9 years ago

Closed 23 months ago

#25105 closed Bug (fixed)

Migrating multiple CharFields to null=False breaks on PostgreSQL

Reported by: Daniel Roseman Owned by: David Wobrock
Component: Migrations Version: 3.2
Severity: Normal Keywords:
Cc: Michał Łazowik, Florian Demmer, David Wobrock Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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 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.

Change History (15)

comment:1 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted

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.

comment:2 by Zanuda999, 9 years ago

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

comment:3 by Daniel Roseman, 9 years ago

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

comment:4 by Luke Plant, 9 years ago

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.

comment:5 by Rafael Ponieman, 8 years ago

Seems to be happening also at least with PositiveIntegerField, even specifying a default when creating the migration.

comment:6 by Michał Łazowik, 6 years ago

Cc: Michał Łazowik added

comment:7 by Simon Charette, 6 years ago

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.

comment:8 by Giovanni Totaro - aka Vanni, 6 years ago

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)

comment:9 by Oskar Haller, 5 years ago

Bug still exists in Django 2.2, PostgreSQL 11

comment:10 by Florian Demmer, 4 years ago

Cc: Florian Demmer added

comment:11 by Stian Jensen, 3 years ago

Version: 1.83.2

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.

comment:12 by Carlton Gibson, 3 years ago

Currently (still) waiting for somebody to propose a patch.

comment:13 by David Wobrock, 23 months ago

Cc: David Wobrock added
Has patch: set
Owner: changed from nobody to David Wobrock
Status: newassigned

Hi there,

Here is a PR that tries to tackle the issue.

Let me know what you think of the approach :)

comment:14 by Mariusz Felisiak, 23 months ago

Triage Stage: AcceptedReady for checkin

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 23 months ago

Resolution: fixed
Status: assignedclosed

In 8f04473a:

Fixed #25105 -- Checked deferred constraints before updating rows on PostgreSQL.

Note: See TracTickets for help on using tickets.
Back to Top