Opened 8 years ago

Closed 6 years ago

Last modified 5 years ago

#25817 closed Bug (fixed)

Unable to rename a field reference in foreign key 'to_field'

Reported by: Simon Kelly Owned by: nobody
Component: Migrations Version: dev
Severity: Normal Keywords: to_field rename
Cc: Simon Charette 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

Steps to reproduce:

  1. Create a model with a foreign key referencing another model's field via the 'to_field' arg.
  2. Generate the initial migration
class Bar(models.Model):
    bar_id = models.CharField(max_length=255, db_index=True, unique=True)

class Bazz(models.Model):
    bar = models.ForeignKey(Bar, to_field='bar_id')
  1. Rename the field referenced in 'to_field' and create a migration for the change

Rename 'bar_id' to 'external_id':

class Bar(models.Model):
    external_id = models.CharField(max_length=255, db_index=True, unique=True)

class Bazz(models.Model):
    bar = models.ForeignKey(Bar, to_field='external_id') 

Migration:

    operations = [
        migrations.RenameField(
            model_name='bar',
            old_name='bar_id',
            new_name='external_id',
        ),
        migrations.AlterField(
            model_name='bazz',
            name='bar',
            field=models.ForeignKey(to='form_processor.Bar', to_field=b'external_id'),
            preserve_default=True,
        ),
    ]
  1. Run the migration

Error:

Traceback (most recent call last):
  File "./manage.py", line 73, in <module>
    execute_from_command_line(sys.argv)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/commands/sqlmigrate.py", line 30, in execute
    return super(Command, self).execute(*args, **options)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/commands/sqlmigrate.py", line 61, in handle
    sql_statements = executor.collect_sql(plan)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 82, in collect_sql
    migration.apply(project_state, schema_editor, collect_sql=True)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/migration.py", line 108, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 139, in database_forwards
    schema_editor.alter_field(from_model, from_field, to_field)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/backends/schema.py", line 445, in alter_field
    old_db_params = old_field.db_parameters(connection=self.connection)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1787, in db_parameters
    return {"type": self.db_type(connection), "check": []}
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1778, in db_type
    rel_field = self.related_field
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1684, in related_field
    return self.foreign_related_fields[0]
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1442, in foreign_related_fields
    return tuple(rhs_field for lhs_field, rhs_field in self.related_fields)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1429, in related_fields
    self._related_fields = self.resolve_related_fields()
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1422, in resolve_related_fields
    else self.rel.to._meta.get_field_by_name(to_field_name)[0])
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/models/options.py", line 420, in get_field_by_name
    % (self.object_name, name))
django.db.models.fields.FieldDoesNotExist: Bar has no field named 'bar_id'

Change History (12)

comment:1 by Simon Kelly, 8 years ago

As a workaround I was able to use the following custom migration:

    operations = [
        migrations.RunSQL(
            'ALTER TABLE form_processor_bar RENAME COLUMN bar_id TO external_id',
            'ALTER TABLE form_processor_bar RENAME COLUMN external_id TO bar_id',
            state_operations=[
                migrations.RenameField(
                    model_name='bar',
                    old_name='bar_id',
                    new_name='external_id',
                ),
                migrations.AlterField(
                    model_name='bazz',
                    name='bar',
                    field=models.ForeignKey(to='form_processor.Bar', to_field=b'external_id'),
                    preserve_default=True,
                ),
            ]
        )
    ]

However this migration is un-sqaushable:

$ ./manage.py squashmigrations form_processor 0002
Will squash the following migrations:
 - 0001_initial
 - 0002_rename_bar_id_to_external_id
Do you wish to proceed? [yN] y
Optimizing...
Traceback (most recent call last):
  File "./manage.py", line 73, in <module>
    execute_from_command_line(sys.argv)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/core/management/commands/squashmigrations.py", line 130, in handle
    fh.write(writer.as_string())
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/writer.py", line 131, in as_string
    operation_string, operation_imports = OperationWriter(operation).serialize()
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/writer.py", line 88, in serialize
    arg_string, arg_imports = MigrationWriter.serialize(arg_value)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/writer.py", line 263, in serialize
    item_string, item_imports = cls.serialize(item)
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/writer.py", line 350, in serialize
    return cls.serialize_deconstructed(*value.deconstruct())
  File "/home/skelly/.virtualenvs/hq/local/lib/python2.7/site-packages/django/db/migrations/writer.py", line 226, in serialize_deconstructed
    module, name = path.rsplit(".", 1)
ValueError: need more than 1 value to unpack
Version 0, edited 8 years ago by Simon Kelly (next)

comment:2 by Simon Kelly, 8 years ago

Type: UncategorizedBug

comment:3 by Simon Charette, 8 years ago

Triage Stage: UnreviewedAccepted
Version: 1.7master

The list of operations generated should look like:

  1. Alter the field Bazz.bar type to an IntegerField in order to drop the FK constraint.
  2. Perform the Bar.bar_id rename to external_id;
  3. Alter back the field Bazz.bar to a FK referring to Bar.external_id;

@snopoke, can you confirm these operations work?

comment:4 by Simon Charette, 8 years ago

Cc: Simon Charette added

in reply to:  4 comment:5 by Simon Kelly, 8 years ago

Replying to charettes:

Yes that does work and I guess its fine for small tables but not great for a big table. Certainly solves my problem right now. Thanks.

comment:6 by Simon Charette, 6 years ago

Has patch: set

PR which still crashes SQLite when foreign key support is enabled which is the default on master.

comment:7 by Tim Graham, 6 years ago

Here's a test from #28305 which doesn't pass because of this issue (I think).

def test_rename_field_reloads_state_on_fk_with_to_field_target_changes(self):
    """
    If RenameField doesn't reload state appropriately, the second AlterField
    crashes on MySQL due to not dropping the PonyRider.slug foreign key
    constraint before modifying the column.
    """
    app_label = 'alter_rename_field_reloads_state_on_fk_with_to_field_target_changes'
    project_state = self.apply_operations(app_label, ProjectState(), operations=[
        migrations.CreateModel('Rider', fields=[
            ('id', models.CharField(primary_key=True, max_length=100)),
            ('slug', models.CharField(unique=True, max_length=100)),
        ]),
        migrations.CreateModel('Pony', fields=[
            ('id', models.CharField(primary_key=True, max_length=100)),
            ('rider', models.ForeignKey('%s.Rider' % app_label, models.CASCADE, to_field='slug')),
            ('slug', models.CharField(unique=True, max_length=100)),
        ]),
        migrations.CreateModel('PonyRider', fields=[
            ('id', models.AutoField(primary_key=True)),
            ('pony', models.ForeignKey('%s.Pony' % app_label, models.CASCADE, to_field='slug')),
        ]),
    ])
    project_state = self.apply_operations(app_label, project_state, operations=[
        migrations.RenameField('Rider', 'slug', 'slug2'),
        migrations.AlterField(
            'Pony', 'rider', models.ForeignKey('%s.Rider' % app_label, models.CASCADE, to_field='slug2')
        ),
        migrations.AlterField('Pony', 'slug', models.CharField(unique=True, max_length=99)),
    ])
    #
    # Crashes with:
    #  File "/home/tim/code/django/tests/migrations/test_operations.py", line 35, in apply_operations
    #    return migration.apply(project_state, editor)
    #  File "/home/tim/code/django/django/db/migrations/migration.py", line 124, in apply
    #    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
    #  File "/home/tim/code/django/django/db/migrations/operations/fields.py", line 216, in database_forwards
    #    schema_editor.alter_field(from_model, from_field, to_field)
    #  File "/home/tim/code/django/django/db/backends/base/schema.py", line 479, in alter_field
    #    old_db_params = old_field.db_parameters(connection=self.connection)
    #  File "/home/tim/code/django/django/db/models/fields/related.py", line 966, in db_parameters
    #    return {"type": self.db_type(connection), "check": self.db_check(connection)}
    #  File "/home/tim/code/django/django/db/models/fields/related.py", line 963, in db_type
    #    return self.target_field.rel_db_type(connection=connection)
    #  File "/home/tim/code/django/django/db/models/fields/related.py", line 877, in target_field
    #    return self.foreign_related_fields[0]
    #  File "/home/tim/code/django/django/db/models/fields/related.py", line 634, in foreign_related_fields
    #    return tuple(rhs_field for lhs_field, rhs_field in self.related_fields if rhs_field)
    #  File "/home/tim/code/django/django/db/models/fields/related.py", line 621, in related_fields
    #    self._related_fields = self.resolve_related_fields()
    #  File "/home/tim/code/django/django/db/models/fields/related.py", line 614, in resolve_related_fields
    #    else self.remote_field.model._meta.get_field(to_field_name))
    #  File "/home/tim/code/django/django/db/models/options.py", line 566, in get_field
    #    raise FieldDoesNotExist("%s has no field named '%s'" % (self.object_name, field_name))
    # django.core.exceptions.FieldDoesNotExist: Rider has no field named 'slug'

comment:8 by Simon Charette, 6 years ago

I added the remaining test coverage for ForeignObject while I was around. The patch should be ready for a final review.

comment:9 by Tim Graham, 6 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In dcdd219e:

Fixed #25817 -- Made RenameField repoint to_field/to_fields references.

Also updated the autodetector to assume the RenameField operation will
perform the required repointing.

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 2839659b:

Fixed #30868 -- Prevented unnecessary AlterField when renaming a referenced pk.

Regression introduced by dcdd219ee1, refs #25817.

Thanks Carlos E. C. Leite for the report and Mariusz for the bisect.

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In bab3ad5:

[3.0.x] Fixed #30868 -- Prevented unnecessary AlterField when renaming a referenced pk.

Regression introduced by dcdd219ee1, refs #25817.

Thanks Carlos E. C. Leite for the report and Mariusz for the bisect.

Backport of 2839659b42ef80038152768b6cedae1016c59d90 from master

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