Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#29000 closed Bug (fixed)

RenameModel does not rename M2M column when run after RenameField

Reported by: David Nelson Adamec Owned by: Jeff
Component: Migrations Version: 2.0
Severity: Normal Keywords:
Cc: 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 David Nelson Adamec)

Encountered this on a project at work. I have some models like so:

class ModelA(models.Model):
    name_renamed = models.CharField(max_length=10)

class ModelBRenamed(models.Model):
    a = models.ForeignKey(ModelA, null=True)

class ModelC(models.Model):
    b_set = models.ManyToManyField(ModelBRenamed)

I have a migration 0002_renamefield to rename a field on ModelA:

class Migration(migrations.Migration):

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

    operations = [
        migrations.RenameField(
            model_name='modela',
            old_name='name',
            new_name='name_renamed',
        ),
    ]

Then a migration 0003_renamemodel to rename ModelB:

class Migration(migrations.Migration):

    dependencies = [
        ('myapp', '0002_renamefield'),
    ]

    operations = [
        migrations.RenameModel(
            old_name='ModelB',
            new_name='ModelBRenamed',
        ),
    ]

If the two migrations are run together, then the modelb_id column in myapp_modelc_b_set will not be renamed to modelbrenamed_id. However, if I run the migrations one at a time, the column will be renamed as expected.

I think this is related to #27737. When ModelA is reloaded in the RenameField migration, ModelB is reloaded due to its foreign key but is missing its relationship to ModelC. When the RenameModel migration is run, ModelC is not included in ModelB's related_objects, so the through table's column is not renamed.

I've reproduced this using Django 1.11 on either MySQL or SQLite.

Change History (13)

comment:1 by Tim Graham, 6 years ago

Component: UncategorizedMigrations
Type: UncategorizedBug

Please test with Django 2.0 as there are changes there which may resolve this.

in reply to:  1 comment:2 by David Nelson Adamec, 6 years ago

Sorry, should have just done that from the start. Tested in Django 2.0.1, and the issue still occurs.

comment:3 by David Nelson Adamec, 6 years ago

Description: modified (diff)
Version: 1.112.0

comment:4 by Tim Graham, 6 years ago

Triage Stage: UnreviewedAccepted

Thanks, good bug report. I can also reproduce on PostgreSQL with Django master (e2908ecb3e20a629be801ddb826c474f7e7023ea).

comment:5 by shangdahao, 6 years ago

Owner: changed from nobody to shangdahao
Status: newassigned

comment:6 by David Nelson Adamec, 6 years ago

After doing a little more digging, it appears that the reload_model with delay=True in the RenameField migration is the culprit. Because of the delay flag, only the original model and its directly related models (ModelA and ModelB in the example) are reloaded, so ModelC is untouched and its b_set field continues to point at the old version of ModelB that's no longer registered. Since ModelC isn't pointing at the new ModelB, it's not included in ModelB's related_objects.

I'm not sure what the performance implication would be if delayed loading were removed from RenameField/AlterField, but I'm sure it's not good. Perhaps there's another clever solution out there, but I feel a bit out of my depth. Here's the workaround I'm using for the time being.

class RenameModelWorkaround(migrations.RenameModel):
    def database_forwards(self, app_label, schema_editor, from_state, to_state):
        from_state.reload_model(app_label, self.old_name_lower)
        super(RenameModelWorkaround, self).database_forwards(
            app_label, schema_editor, from_state, to_state)

comment:7 by shangdahao, 6 years ago

Owner: shangdahao removed
Status: assignednew

comment:8 by Jeff, 6 years ago

Owner: set to Jeff
Status: newassigned

Unsurprisingly, this bug also breaks backwards migrations, as the names in the bridge table don't match expected values.

For the models/migrations in the tickets description, backwards migrations produces the expected result:

Traceback (most recent call last):
  File "manage.py", line 15, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/core/management/base.py", line 294, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/core/management/base.py", line 331, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/core/management/base.py", line 83, in wrapped
    res = handle_func(*args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/core/management/commands/migrate.py", line 203, in handle
    fake_initial=fake_initial,
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/migrations/executor.py", line 121, in migrate
    state = self._migrate_all_backwards(plan, full_plan, fake=fake)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/migrations/executor.py", line 196, in _migrate_all_backwards
    self.unapply_migration(states[migration], migration, fake=fake)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/migrations/executor.py", line 262, in unapply_migration
    state = migration.unapply(state, schema_editor)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/migrations/migration.py", line 175, in unapply
    operation.database_backwards(self.app_label, schema_editor, from_state, to_state)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/migrations/operations/models.py", line 377, in database_backwards
    self.database_forwards(app_label, schema_editor, from_state, to_state)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/migrations/operations/models.py", line 349, in database_forwards
    to_field,
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/base/schema.py", line 508, in alter_field
    return self._alter_many_to_many(model, old_field, new_field, strict)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/base/schema.py", line 863, in _alter_many_to_many
    new_field.remote_field.through._meta.get_field(new_field.m2m_reverse_field_name()),
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/base/schema.py", line 523, in alter_field
    old_db_params, new_db_params, strict)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/base/schema.py", line 605, in _alter_field
    self.execute(self._rename_field_sql(model._meta.db_table, old_field, new_field, new_type))
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/base/schema.py", line 133, in execute
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/utils.py", line 100, in execute
    return super().execute(sql, params)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/utils.py", line 68, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/utils.py", line 77, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.6/dist-packages/Django-2.1.dev20180514000620-py3.6.egg/django/db/backends/mysql/base.py", line 71, in execute
    return self.cursor.execute(query, args)
  File "/usr/lib/python3/dist-packages/MySQLdb/cursors.py", line 250, in execute
    self.errorhandler(self, exc, value)
  File "/usr/lib/python3/dist-packages/MySQLdb/connections.py", line 50, in defaulterrorhandler
    raise errorvalue
  File "/usr/lib/python3/dist-packages/MySQLdb/cursors.py", line 247, in execute
    res = self._query(query)
  File "/usr/lib/python3/dist-packages/MySQLdb/cursors.py", line 411, in _query
    rowcount = self._do_query(q)
  File "/usr/lib/python3/dist-packages/MySQLdb/cursors.py", line 374, in _do_query
    db.query(q)
  File "/usr/lib/python3/dist-packages/MySQLdb/connections.py", line 292, in query
    _mysql.connection.query(self, query)
django.db.utils.OperationalError: (1054, "Unknown column 'modelbrenamed_id' in 'temp_modelc_b_set'")

Version 0, edited 6 years ago by Jeff (next)

comment:10 by Jeff, 6 years ago

Has patch: set
Last edited 6 years ago by Tim Graham (previous) (diff)

comment:11 by Tim Graham, 6 years ago

Summary: RenameModel does not rename M2M column when run after AlterField/RenameFieldRenameModel does not rename M2M column when run after RenameField
Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In fcc4e251:

Fixed #29000 -- Fixed RenameModel's renaming of a M2M column when run after RenameField.

Regression in 45ded053b1f4320284aa5dac63052f6d1baefea9.

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

In 2b7b1993:

[2.1.x] Fixed #29000 -- Fixed RenameModel's renaming of a M2M column when run after RenameField.

Regression in 45ded053b1f4320284aa5dac63052f6d1baefea9.

Backport of fcc4e251dbc917118f73d7187ee2f4cbf3883f36 from master

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In ad81133:

Refs #29000 -- Restored delayed model rendering of RenameField.

Non-delayed rendering is unnecessary and wasteful now that state models
relationship consistency on delayed reload is ensured.

This partly reverts commit fcc4e251dbc917118f73d7187ee2f4cbf3883f36.

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