Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#24630 closed Bug (fixed)

Misleading/incorrect docs about RunPython and transactions

Reported by: Luca Corti Owned by: priidukull
Component: Documentation Version: 1.8
Severity: Normal Keywords: uuidfiled migrations
Cc: priidukull 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 (last modified by Luca Corti)

As per #23932, I added an UUIDField to a model following the documentation at https://docs.djangoproject.com/en/1.8/howto/writing-migrations/. The database is PostgreSQL.

When running the migration I get this error:

  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "venv/lib/python2.7/site-packages/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "venv/lib/python2.7/site-packages/django/core/management/__init__.py", line 330, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "venv/lib/python2.7/site-packages/django/core/management/base.py", line 390, in run_from_argv
    self.execute(*args, **cmd_options)
  File "venv/lib/python2.7/site-packages/django/core/management/base.py", line 441, in execute
    output = self.handle(*args, **options)
  File "venv/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 221, in handle
    executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
  File "venv/lib/python2.7/site-packages/django/db/migrations/executor.py", line 110, in migrate
    self.apply_migration(states[migration], migration, fake=fake, fake_initial=fake_initial)
  File "venv/lib/python2.7/site-packages/django/db/migrations/executor.py", line 147, in apply_migration
    state = migration.apply(state, schema_editor)
  File "venv/lib/python2.7/site-packages/django/db/migrations/migration.py", line 115, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "venv/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 201, in database_forwards
    schema_editor.alter_field(from_model, from_field, to_field)
  File "venv/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 484, in alter_field
    old_db_params, new_db_params, strict)
  File "venv/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 637, in _alter_field
    params,
  File "venv/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 107, in execute
    cursor.execute(sql, params)
  File "venv/lib/python2.7/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "venv/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "venv/lib/python2.7/site-packages/django/db/utils.py", line 97, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "venv/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
django.db.utils.OperationalError: cannot ALTER TABLE "mytable" because it has pending trigger events

Change History (15)

comment:1 Changed 3 years ago by Luca Corti

Description: modified (diff)

comment:2 Changed 3 years ago by Kévin Etienne

I've run into what I believe the same issue with Django 1.7.x. I have a model where I wanted to add and remove a new field.

The migration had 3 operations:

  • add a new field
  • migrate the data
  • and remove the old field.
...
    operations = [
        migrations.AddField(
            model_name='model',
            name='new_field',
            field=models.BooleanField(default=False),
            preserve_default=True,
        ),
        migrations.RunPython(
            populate_new_field,
            populate_old_field,
        ),
        migrations.RemoveField(
            model_name='model',
            name='old_field',
        ),
    ]

The stacktrace looks exactly the same as quote in the original description. Not sure if this is a behavior expected from Django or not. The documentation on 1.7 and 1.8 states that RunPython should run into its own transaction.

This is happening when the table has already some data but not when the table is empty.

Last edited 3 years ago by Kévin Etienne (previous) (diff)

comment:3 in reply to:  2 Changed 3 years ago by KwonHan Bae

your database is postgresql?

comment:4 Changed 3 years ago by priidukull

Owner: changed from nobody to priidukull
Status: newassigned

comment:5 Changed 3 years ago by priidukull

I tried to reproduce the bug (as explained by KevinEtienne) with Python 2.7 and Django 1.7/1.8. Did not manage to reproduce the issue. I used the following migrations.

0001_initial.py:

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


class Migration(migrations.Migration):

    dependencies = [
    ]

    operations = [
        migrations.CreateModel(
            name='Choice',
            fields=[
                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
                ('choice_text', models.CharField(max_length=200)),
                ('votes', models.IntegerField(default=0)),
            ],
        ),
        migrations.CreateModel(
            name='Question',
            fields=[
                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
                ('question_text', models.CharField(max_length=200)),
                ('pub_date', models.DateTimeField(verbose_name=b'date published')),
            ],
        ),
        migrations.AddField(
            model_name='choice',
            name='question',
            field=models.ForeignKey(to='polls.Question'),
        ),
    ]

0002_second.py:

# -*- coding: utf-8 -*-
from __future__ import unicode_literals
import random

from django.db import migrations


def populate_old_field(apps, schema_editor):
    Question = apps.get_model('polls', 'Question')
    for i in range(1, 100):
        q = Question(i, random.getrandbits(128), '2015-09-09')
        q.save()


class Migration(migrations.Migration):

    dependencies = [
        ('polls', '0001_initial'),
    ]

    operations = [
        migrations.RunPython(
            populate_old_field
        ),
    ]

0003_third.py:

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


def populate_new_field(apps, schema_editor):
    Question = apps.get_model('polls', 'Question')
    for q in Question.objects.all():
        q.question_text_new = q.question_text
        q.save()


class Migration(migrations.Migration):

    dependencies = [
        ('polls', '0002_second'),
    ]

    operations = [
        migrations.AddField(
            model_name='question',
            name='question_text_new',
            field=models.CharField(max_length=300, null=True),
        ),
        migrations.RunPython(
            populate_new_field,
        ),
        migrations.RemoveField(
            model_name='question',
            name='question_text',
        ),
    ]
Last edited 3 years ago by priidukull (previous) (diff)

comment:6 Changed 3 years ago by priidukull

Resolution: needsinfo
Status: assignedclosed

comment:7 Changed 3 years ago by priidukull

Cc: priidukull added

comment:8 Changed 3 years ago by ollanta

Hi, I've stumbled across this issue as well.
I'm not getting the error with your example either priidukull, so I reduced the code I had to a small example that still exhibits the bug (in both django 1.7 and 1.8, but only in psql, not sqlite)

0001_initial.py :

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations
from django.conf import settings


class Migration(migrations.Migration):

    dependencies = []

    operations = [
        migrations.CreateModel(
            name='Parent',
            fields=[
                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
            ],
        ),
        migrations.CreateModel(
            name='Child',
            fields=[
                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
                ('boolfield', models.BooleanField(default=True)),
                ('parent', models.ForeignKey(to='example.Parent')),
            ],
        ),

    ]

0002_add_data.py:

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


def add_data(apps, schema_editor):
    Parent = apps.get_model("example", "Parent")
    p = Parent(1)
    p.save()

    Child = apps.get_model("example", "Child")
    c = Child(1, parent=p, boolfield=True)
    c.save()


class Migration(migrations.Migration):

    dependencies = [
        ('example', '0001_initial'),
    ]

    operations = [
        migrations.RunPython(
            add_data
        )
    ]

0003_change_field_type.py:

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


def convert_field(apps, schema_editor):
    Child = apps.get_model("example", "Child")
    for c in Child.objects.all():
        c.textchoicefield = 'true' if c.boolfield else 'false'
        c.save()


class Migration(migrations.Migration):

    dependencies = [
        ('example', '0002_add_data'),
    ]

    operations = [
        migrations.AddField(
            model_name='child',
            name='textchoicefield',
            field=models.TextField(default=b'none', choices=[(b'true', b'True'), (b'false', b'False'), (b'none', b'None')]),
            preserve_default=True,
        ),
        migrations.RunPython(convert_field),
        migrations.RemoveField(
            model_name='child',
            name='boolfield',
        ),
    ]

If I move RemoveField out of the last migration and into its own, it all works fine.

comment:9 Changed 3 years ago by Tim Graham

Resolution: needsinfo
Status: closednew

comment:10 Changed 3 years ago by Tim Graham

Component: MigrationsDocumentation
Severity: NormalRelease blocker
Summary: UUIDField migrationMisleading/incorrect docs about RunPython and transactions
Triage Stage: UnreviewedAccepted

It seems to me that the documentation that says, "By default, RunPython will run its contents inside a transaction" is misleading (added in 5a917cfef319df33ca30a3b27bd0b0533b8e63bb).

Each *migration* is run in its own transaction (if the database backend has can_rollback_ddl = True) because we use a SchemaEditor context manager when applying each migration, which creates a transaction.

I'm not sure we can make any changes code-wise to support mixing schema changes and RunPython operations in the same migration, but we should correct the documentation for current version of Django anyway.

Docs to fix:

comment:11 Changed 3 years ago by Tim Graham

Has patch: set

comment:12 Changed 3 years ago by Tim Graham

Severity: Release blockerNormal

comment:13 Changed 3 years ago by Markus Holtermann

Triage Stage: AcceptedReady for checkin

comment:14 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 307acc7:

Fixed #24630 -- Clarified docs about RunPython transactions.

Thanks Markus Holtermann for review.

comment:15 Changed 3 years ago by Tim Graham <timograham@…>

In cc4ee062:

[1.8.x] Fixed #24630 -- Clarified docs about RunPython transactions.

Thanks Markus Holtermann for review.

Backport of 307acc745a4e655c35db96f96ceb4b87597dee49 from master

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