Migrations crash renaming the pk of a model with a self-referential foreign key on SQLite

[Brian May]

With open source project called spud.

Generates Exception:

FieldDoesNotExist: album has no field named u'album_id'

This field was removed. However in the migration that is being run when it fails, it did exist. So as a result, I suspect this isn't #26180.

The migration in question attempts to rename the album_id field to id.

Works fine with Django < =1.8

Stack Trace:

/usr/lib/python2.7/dist-packages/pytest_django/fixtures.py:54: in _django_db_setup
    interactive=False)
/tmp/deleteme/local/lib/python2.7/site-packages/django/test/runner.py:726: in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/backends/base/creation.py:70: in create_test_db
    run_syncdb=True,
/tmp/deleteme/local/lib/python2.7/site-packages/django/core/management/__init__.py:119: in call_command
    return command.execute(*args, **defaults)
/tmp/deleteme/local/lib/python2.7/site-packages/django/core/management/base.py:399: in execute
    output = self.handle(*args, **options)
/tmp/deleteme/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py:200: in handle
    executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/migrations/executor.py:92: in migrate
    self._migrate_all_forwards(plan, full_plan, fake=fake, fake_initial=fake_initial)
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/migrations/executor.py:121: in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/migrations/executor.py:198: in apply_migration
    state = migration.apply(state, schema_editor)
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/migrations/migration.py:123: in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/migrations/operations/fields.py:275: in database_forwards
    to_model._meta.get_field(self.new_name),
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/backends/base/schema.py:482: in alter_field
    old_db_params, new_db_params, strict)
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/backends/sqlite3/schema.py:245: in _alter_field
    self._remake_table(model, alter_fields=[(old_field, new_field)])
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/backends/sqlite3/schema.py:181: in _remake_table
    self.create_model(temp_model)
/tmp/deleteme/local/lib/python2.7/site-packages/django/db/backends/base/schema.py:250: in create_model
    to_column = field.remote_field.model._meta.get_field(field.remote_field.field_name).column

Actual error is in get_field() at

/tmp/deleteme/local/lib/python2.7/site-packages/django/db/models/options.py:582: FieldDoesNotExist

The migration that appears to be causing the problems (wish I could tell which step was failing):

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

from django.db import models, migrations


class Migration(migrations.Migration):

    dependencies = [
        ('spud', '0003_auto_20141217_0908'),
    ]

    operations = [
        migrations.AlterModelOptions(
            name='photo',
            options={'ordering': ['datetime', 'id']},
        ),
        migrations.RenameField(
            model_name='album',
            old_name='album_id',
            new_name='id',
        ),
        migrations.RenameField(
            model_name='category',
            old_name='category_id',
            new_name='id',
        ),
        migrations.RenameField(
            model_name='person',
            old_name='person_id',
            new_name='id',
        ),
        migrations.RenameField(
            model_name='photo',
            old_name='photo_id',
            new_name='id',
        ),
        migrations.RenameField(
            model_name='place',
            old_name='place_id',
            new_name='id',
        ),
    ]

[Brian May]

Code that causes this failure on ​github.

[Brian May]

Looks like the above migration fails for sqlite (above error), fails for mysql (different error), but works for postgresql. So to try and summarize:

I said earlier this works for Python3.4, doesn't appear to be the case any more; it fails regardless of Python version. So maybe I missed up the earlier test.

So to get this fail:

• Use mysql (different error) OR
• Use sqlite and Django >= 1.9 (error quoted above)

Postgresql appears to work with any Django version. So maybe I somehow accidentally made the migration postgresql specific???

Test results available here: ​https://travis-ci.org/brianmay/spud/builds/117384657

[Brian May]

Suspect the failure on mysql is that mysql doesn't like having the primary key renamed.

The failure with sqlite looks like some sort of regression in Django 1.9. No idea if is it feasible to fix or not.

Apologies if any of this was documented somewhere and I missed it...

I plan to work around this for now by squashing the migrations and removing the rename.

[Tim Graham]

Bisected the behavior change to 720ff740e70e649a97fcf0232fec132569a52c7e. Haven't confirmed it's a bug in Django and not in the application but accepting for further investigation.

If you could create a more minimal set of steps to reproduce the issue, that would be helpful.

[Tim Graham]

I'm attaching a minimal project to reproduce. Add the rename app to a project and run python manage.py test rename and you should see FieldDoesNotExist: Foo has no field named u'album_id' (reproducible with SQLite only). If someone can propose a fix, I don't mind spending more time trying to write a regression test but as I'm not sure where the fix lies, I'm not sure at what level to write a test.

[Tim Graham]

Alex, could you investigate as you're the author on the patch where the behavior changed?

[Alex Hill]

Sure. I'm away for Easter weekend with just phone, I'll have a look next week.

I reckon it will have something to do with the issues described in ​https://github.com/django/django/pull/4122

[Alex Hill]

The model in Tim's test case looks like this:

class Foo(models.Model):
    # album_id = models.AutoField(primary_key=True) (renamed to `id` in migration 2)
    parent = models.ForeignKey('self', null=True, blank=True)

The problematic migration wants to rename Foo.album_id to Foo.id. To perform this kind of schema alteration under SQLite, we have to create a new model with updated fields, and from that model make a new table, copy the data, then remove the old table. The action happens in ​db.backends.sqlite3.schema.DatabaseSchemaEditor._remake_table().

The bug occurs because only the AutoField is changed, while the ForeignKey is considered not to have changed and is copied directly from the pre-migration model. In fact the ForeignKey, in its remote_field.field_name, holds a copy of the AutoField's field name, so it needs to be updated too to ensure the temporary model is internally consistent.

To fix the bug we can re-create a model's self-referential related fields using Field.clone() rather than just using the old model's field instances.

I've opened a PR at ​https://github.com/django/django/pull/6355.

[Tim Graham <timograham@…>]

In 4b2cf1c:

Fixed #26384 -- Fixed renaming the PK on a model with a self-referential FK on SQLite.

[Tim Graham <timograham@…>]

In ed87af3:

[1.9.x] Fixed #26384 -- Fixed renaming the PK on a model with a self-referential FK on SQLite.

Backport of 4b2cf1cd27587a30b3b081091627d7ee13141afe from master

[Claude Paroz]

This appears to cause failures with MySQL (5.5?): ​http://djangoci.com/job/django-master/

[Alex Hill]

Hi Claude,

Interesting...my best guess is that this is a separate problem exposed by the test that was added in this patch, since the only code changes in the patch were to the SQLite schema editor. The original report bears that out:

So to get this fail:

• Use mysql (different error) OR
• Use sqlite and Django >= 1.9 (error quoted above)

I don't have MySQL set up, but this will be useful: ​http://stackoverflow.com/questions/1457305/mysql-creating-tables-with-foreign-keys-giving-errno-150

That said, the pull request passed all the tests under all databases. How could that have happened?

Cheers,
Alex

[Alex Hill]

I think this is the problem:

Prior to MySQL 5.6.6, adding and dropping a foreign key in the same ALTER TABLE statement may be problematic in some cases and is therefore unsupported. Separate statements should be used for each operation. As of MySQL 5.6.6, adding and dropping a foreign key in the same ALTER TABLE statement is supported for ALTER TABLE ... ALGORITHM=INPLACE but remains unsupported for ALTER TABLE ... ALGORITHM=COPY.

From ​http://dev.mysql.com/doc/refman/5.6/en/alter-table.html

[Claude Paroz]

That said, the pull request passed all the tests under all databases. How could that have happened?

Because the main pull request builder is using a recent MySQL (unless you ask the buildbot to test on precise).

It's true that you probably revealed an existing bug with older MySQL versions. Do you have an idea for fixing this one?

[Tim Graham]

I think the MySQL issue is #24995 ("MySQL error when renaming a primary key"). There is a patch there but I closed the PR because the test didn't act as a regression test for me. Now I see that maybe I needed to test with an older MySQL. The patch there still doesn't solve the issue but it changes the exception on the test added in this ticket to something within Django AttributeError: 'AutoField' object has no attribute 'model' instead of something from MySQL: OperationalError: (1025, "Error on rename of './test_django/#sql-58c_94' to './test_django/schema_node' (errno: 150)"). I think we can continue the discussion on that ticket.

[Tim Graham <timograham@…>]

In f3595b2:

Refs #26384, #24995 -- Skipped a schema test on older MySQL versions.

[Tim Graham <timograham@…>]

In a0e3cbaa:

[1.9.x] Refs #26384, #24995 -- Skipped a schema test on older MySQL versions.

Backport of f3595b25496691966d4ff858a3b395735ad85a6e from master

[Tim Graham <timograham@…>]

In d81d02d:

Refs #26384, #24995 -- Avoided a module-level MySQL query in the schema tests.

[Tim Graham <timograham@…>]

In 5c1944f9:

[1.9.x] Refs #26384, #24995 -- Avoided a module-level MySQL query in the schema tests.

Backport of d81d02d449edd046a94de5f171f4ae87fa331c7d from master

[Simon Charette <charette.s@…>]

In 96181080:

Refs #26384 -- Isolated a test model in schema tests.