From c3e0adcad8d8ba94b33cabd137056166ed36dae0 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Fri, 30 Jun 2017 14:51:08 -0400 Subject: [PATCH] Fixed #28305 -- Fixed "Cannot change column 'x': used in a foreign key constraint" crash on MySQL with a sequence of AlterField or RenameField operations. Regression in 45ded053b1f4320284aa5dac63052f6d1baefea9. --- django/db/backends/base/schema.py | 14 +++-- django/db/migrations/operations/fields.py | 17 +++-- django/db/migrations/operations/utils.py | 9 +++ docs/releases/1.11.8.txt | 4 ++ tests/migrations/test_operations.py | 77 +++++++++++++++++++++++ 5 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 django/db/migrations/operations/utils.py diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 35b82dc1e52..36e7b503420 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -536,9 +536,15 @@ class BaseDatabaseSchemaEditor: )) for constraint_name in constraint_names: self.execute(self._delete_constraint_sql(self.sql_delete_unique, model, constraint_name)) - # Drop incoming FK constraints if we're a primary key and things are going - # to change. - if old_field.primary_key and new_field.primary_key and old_type != new_type: + # Drop incoming FK constraints if the field is a primary key or unique, + # which might be a to_field target, and things are going to change. + drop_foreign_keys = ( + ( + (old_field.primary_key and new_field.primary_key) or + (old_field.unique and new_field.unique) + ) and old_type != new_type + ) + if drop_foreign_keys: # '_meta.related_field' also contains M2M reverse fields, these # will be filtered out for _old_rel, new_rel in _related_non_m2m_objects(old_field, new_field): @@ -727,7 +733,7 @@ class BaseDatabaseSchemaEditor: new_field.db_constraint): self.execute(self._create_fk_sql(model, new_field, "_fk_%(to_table)s_%(to_column)s")) # Rebuild FKs that pointed to us if we previously had to drop them - if old_field.primary_key and new_field.primary_key and old_type != new_type: + if drop_foreign_keys: for rel in new_field.model._meta.related_objects: if not rel.many_to_many and rel.field.db_constraint: self.execute(self._create_fk_sql(rel.related_model, rel.field, "_fk")) diff --git a/django/db/migrations/operations/fields.py b/django/db/migrations/operations/fields.py index b2c552c5461..d103e5c127c 100644 --- a/django/db/migrations/operations/fields.py +++ b/django/db/migrations/operations/fields.py @@ -3,6 +3,7 @@ from django.db.models.fields import NOT_PROVIDED from django.utils.functional import cached_property from .base import Operation +from .utils import is_referenced_by_foreign_key class FieldOperation(Operation): @@ -196,8 +197,12 @@ class AlterField(FieldOperation): ] # TODO: investigate if old relational fields must be reloaded or if it's # sufficient if the new field is (#27737). - # Delay rendering of relationships if it's not a relational field - delay = not field.is_relation + # Delay rendering of relationships if it's not a relational field and + # not referenced by a foreign key. + delay = ( + not field.is_relation and + not is_referenced_by_foreign_key(state, self.model_name_lower, self.field, self.name) + ) state.reload_model(app_label, self.model_name_lower, delay=delay) def database_forwards(self, app_label, schema_editor, from_state, to_state): @@ -268,8 +273,12 @@ class RenameField(FieldOperation): for index, (name, field) in enumerate(fields): if name == self.old_name: fields[index] = (self.new_name, field) - # Delay rendering of relationships if it's not a relational field. - delay = not field.is_relation + # Delay rendering of relationships if it's not a relational + # field and not referenced by a foreign key. + delay = ( + not field.is_relation and + not is_referenced_by_foreign_key(state, self.model_name_lower, field, self.name) + ) break else: raise FieldDoesNotExist( diff --git a/django/db/migrations/operations/utils.py b/django/db/migrations/operations/utils.py new file mode 100644 index 00000000000..af23ea95634 --- /dev/null +++ b/django/db/migrations/operations/utils.py @@ -0,0 +1,9 @@ +def is_referenced_by_foreign_key(state, model_name_lower, field, field_name): + for state_app_label, state_model in state.models: + for _, f in state.models[state_app_label, state_model].fields: + if (f.related_model and + '%s.%s' % (state_app_label, model_name_lower) == f.related_model.lower() and + hasattr(f, 'to_fields')): + if (f.to_fields[0] is None and field.primary_key) or field_name in f.to_fields: + return True + return False diff --git a/docs/releases/1.11.8.txt b/docs/releases/1.11.8.txt index 596ea434ece..9ef7d73abfe 100644 --- a/docs/releases/1.11.8.txt +++ b/docs/releases/1.11.8.txt @@ -30,3 +30,7 @@ Bugfixes * Fixed a regression in caching of a ``GenericForeignKey`` when the referenced model instance uses multi-table inheritance (:ticket:`28856`). + +* Fixed "Cannot change column 'x': used in a foreign key constraint" crash on + MySQL with a sequence of ``AlterField`` and/or ``RenameField`` operations in + a migration (:ticket:`28305`). diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 13b511f3aac..02f8713650e 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1322,6 +1322,83 @@ class OperationTests(OperationTestBase): operation.database_backwards("test_alflpkfk", editor, new_state, project_state) assertIdTypeEqualsFkType() + def test_alter_field_reloads_state_on_fk_target_changes(self): + """ + If AlterField doesn't reload state appropriately, the second AlterField + crashes on MySQL due to not dropping the PonyRider.pony foreign key + constraint before modifying the column. + """ + app_label = 'alter_alter_field_reloads_state_on_fk_target_changes' + project_state = self.apply_operations(app_label, ProjectState(), operations=[ + migrations.CreateModel('Rider', fields=[ + ('id', models.CharField(primary_key=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)), + ]), + migrations.CreateModel('PonyRider', fields=[ + ('id', models.AutoField(primary_key=True)), + ('pony', models.ForeignKey('%s.Pony' % app_label, models.CASCADE)), + ]), + ]) + project_state = self.apply_operations(app_label, project_state, operations=[ + migrations.AlterField('Rider', 'id', models.CharField(primary_key=True, max_length=99)), + migrations.AlterField('Pony', 'id', models.CharField(primary_key=True, max_length=99)), + ]) + + def test_alter_field_reloads_state_on_fk_with_to_field_target_changes(self): + """ + If AlterField doesn't reload state appropriately, the second AlterField + crashes on MySQL due to not dropping the PonyRider.pony foreign key + constraint before modifying the column. + """ + app_label = 'alter_alter_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.AlterField('Rider', 'slug', models.CharField(unique=True, max_length=99)), + migrations.AlterField('Pony', 'slug', models.CharField(unique=True, max_length=99)), + ]) + + def test_rename_field_reloads_state_on_fk_target_changes(self): + """ + If RenameField doesn't reload state appropriately, the AlterField + crashes on MySQL due to not dropping the PonyRider.pony foreign key + constraint before modifying the column. + """ + app_label = 'alter_rename_field_reloads_state_on_fk_target_changes' + project_state = self.apply_operations(app_label, ProjectState(), operations=[ + migrations.CreateModel('Rider', fields=[ + ('id', models.CharField(primary_key=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)), + ]), + migrations.CreateModel('PonyRider', fields=[ + ('id', models.AutoField(primary_key=True)), + ('pony', models.ForeignKey('%s.Pony' % app_label, models.CASCADE)), + ]), + ]) + project_state = self.apply_operations(app_label, project_state, operations=[ + migrations.RenameField('Rider', 'id', 'id2'), + migrations.AlterField('Pony', 'id', models.CharField(primary_key=True, max_length=99)), + ]) + def test_rename_field(self): """ Tests the RenameField operation.