From 5896aa8367d545c60bb544c31d7ecaaecc321045 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Fri, 15 Oct 2021 13:11:02 -0400 Subject: [PATCH] Fixed #33197 -- Made field rename with prior matching db_column change a noop. Thanks Jacob Walls for the report. --- django/db/migrations/autodetector.py | 19 ++++++++++++++++- django/db/migrations/operations/fields.py | 6 +++++- tests/migrations/test_autodetector.py | 26 ++++++++++++++--------- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 3cd8a43084..d3ac1c93ab 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -840,6 +840,20 @@ class MigrationAutodetector: old_field_dec[0:2] == field_dec[0:2] and dict(old_field_dec[2], db_column=old_db_column) == field_dec[2])): if self.questioner.ask_rename(model_name, rem_field_name, field_name, field): + # A db_column mismatch requires a prior noop + # AlterField for the subsequent RenameField to be a + # noop on attempts at preserving the old name. + if old_field.db_column != field.db_column: + altered_field = field.clone() + altered_field.name = rem_field_name + self.add_operation( + app_label, + operations.AlterField( + model_name=model_name, + name=rem_field_name, + field=altered_field, + ), + ) self.add_operation( app_label, operations.RenameField( @@ -970,7 +984,10 @@ class MigrationAutodetector: new_field.remote_field.through = old_field.remote_field.through old_field_dec = self.deep_deconstruct(old_field) new_field_dec = self.deep_deconstruct(new_field) - if old_field_dec != new_field_dec: + # If the field was confirmed to be renamed it means that only + # db_column was allowed to change which generate_renamed_fields() + # already accounts for by adding an AlterField operation. + if old_field_dec != new_field_dec and old_field_name == field_name: both_m2m = old_field.many_to_many and new_field.many_to_many neither_m2m = not old_field.many_to_many and not new_field.many_to_many if both_m2m or neither_m2m: diff --git a/django/db/migrations/operations/fields.py b/django/db/migrations/operations/fields.py index 641c142191..094c3e3cda 100644 --- a/django/db/migrations/operations/fields.py +++ b/django/db/migrations/operations/fields.py @@ -239,7 +239,11 @@ class AlterField(FieldOperation): def reduce(self, operation, app_label): if isinstance(operation, RemoveField) and self.is_same_field_operation(operation): return [operation] - elif isinstance(operation, RenameField) and self.is_same_field_operation(operation): + elif ( + isinstance(operation, RenameField) and + self.is_same_field_operation(operation) and + self.field.db_column is None + ): return [ operation, AlterField( diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index ed2398ebaf..8fb4897f0e 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -1001,14 +1001,17 @@ class AutodetectorTests(TestCase): ] changes = self.get_changes(before, after, MigrationQuestioner({'ask_rename': True})) self.assertNumberMigrations(changes, 'app', 1) - self.assertOperationTypes(changes, 'app', 0, ['RenameField', 'AlterField']) + self.assertOperationTypes(changes, 'app', 0, ['AlterField', 'RenameField']) self.assertOperationAttributes( - changes, 'app', 0, 0, model_name='foo', old_name='field', new_name='renamed_field', + changes, 'app', 0, 0, model_name='foo', name='field', ) - self.assertOperationAttributes(changes, 'app', 0, 1, model_name='foo', name='renamed_field') - self.assertEqual(changes['app'][0].operations[-1].field.deconstruct(), ( - 'renamed_field', 'django.db.models.IntegerField', [], {'db_column': 'field'}, + self.assertEqual(changes['app'][0].operations[0].field.deconstruct(), ( + 'field', 'django.db.models.IntegerField', [], {'db_column': 'field'}, )) + self.assertOperationAttributes( + changes, 'app', 0, 1, model_name='foo', old_name='field', + new_name='renamed_field', + ) def test_rename_related_field_preserved_db_column(self): before = [ @@ -1031,17 +1034,20 @@ class AutodetectorTests(TestCase): ] changes = self.get_changes(before, after, MigrationQuestioner({'ask_rename': True})) self.assertNumberMigrations(changes, 'app', 1) - self.assertOperationTypes(changes, 'app', 0, ['RenameField', 'AlterField']) + self.assertOperationTypes(changes, 'app', 0, ['AlterField', 'RenameField']) self.assertOperationAttributes( - changes, 'app', 0, 0, model_name='bar', old_name='foo', new_name='renamed_foo', + changes, 'app', 0, 0, model_name='bar', name='foo', ) - self.assertOperationAttributes(changes, 'app', 0, 1, model_name='bar', name='renamed_foo') - self.assertEqual(changes['app'][0].operations[-1].field.deconstruct(), ( - 'renamed_foo', + self.assertEqual(changes['app'][0].operations[0].field.deconstruct(), ( + 'foo', 'django.db.models.ForeignKey', [], {'to': 'app.foo', 'on_delete': models.CASCADE, 'db_column': 'foo_id'}, )) + self.assertOperationAttributes( + changes, 'app', 0, 1, model_name='bar', old_name='foo', + new_name='renamed_foo', + ) def test_rename_model(self): """Tests autodetection of renamed models."""