diff --git a/AUTHORS b/AUTHORS index bfba0c0523..909149137d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -382,6 +382,7 @@ answer newbie questions, and generally made Django that much better: Jensen Cochran Jeong-Min Lee Jérémie Blaser + Jeremy Bowman Jeremy Carbaugh Jeremy Dunck Jeremy Lainé diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index bd07385672..cf18e7653e 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -14,12 +14,28 @@ from django.utils.encoding import force_bytes logger = logging.getLogger('django.db.backends.schema') +def _is_relevant_relation(relation, altered_field): + """ + When altering the given field, must constraints on its model from the given + relation be temporarily dropped? + """ + field = relation.field + if field.many_to_many: + # M2M reverse field + return False + if altered_field.primary_key and field.to_fields == [None]: + # Foreign key constraint on the primary key, which is being altered. + return True + # Is the constraint targeting the field being altered? + return altered_field.name in field.to_fields + + def _related_non_m2m_objects(old_field, new_field): # Filter out m2m objects from reverse relations. # Return (old_relation, new_relation) tuples. return zip( - (obj for obj in old_field.model._meta.related_objects if not obj.field.many_to_many), - (obj for obj in new_field.model._meta.related_objects if not obj.field.many_to_many) + (obj for obj in old_field.model._meta.related_objects if _is_relevant_relation(obj, old_field)), + (obj for obj in new_field.model._meta.related_objects if _is_relevant_relation(obj, new_field)) ) @@ -731,7 +747,7 @@ class BaseDatabaseSchemaEditor: # Rebuild FKs that pointed to us if we previously had to drop them if drop_foreign_keys: for rel in new_field.model._meta.related_objects: - if not rel.many_to_many and rel.field.db_constraint: + if _is_relevant_relation(rel, new_field) and rel.field.db_constraint: self.execute(self._create_fk_sql(rel.related_model, rel.field, "_fk")) # Does it have check constraints we need to add? if old_db_params['check'] != new_db_params['check'] and new_db_params['check']: diff --git a/docs/releases/1.11.13.txt b/docs/releases/1.11.13.txt index 8dbd651652..b9fd3329ef 100644 --- a/docs/releases/1.11.13.txt +++ b/docs/releases/1.11.13.txt @@ -9,4 +9,6 @@ Django 1.11.13 fixes several bugs in 1.11.12. Bugfixes ======== -* ... +* Fixed a regression in Django 1.11.8 where altering a field with a unique + constraint may drop and rebuild more foreign keys than necessary + (:ticket:`29193`). diff --git a/docs/releases/2.0.5.txt b/docs/releases/2.0.5.txt index c7a7a84d9e..c88135aeb9 100644 --- a/docs/releases/2.0.5.txt +++ b/docs/releases/2.0.5.txt @@ -11,3 +11,7 @@ Bugfixes * Corrected the import paths that ``inspectdb`` generates for ``django.contrib.postgres`` fields (:ticket:`29307`). + +* Fixed a regression in Django 1.11.8 where altering a field with a unique + constraint may drop and rebuild more foreign keys than necessary + (:ticket:`29193`). diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 12c33a8433..b535a834f9 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -1545,6 +1545,40 @@ class SchemaTests(TransactionTestCase): TagUniqueRename.objects.create(title="bar", slug2="foo") Tag.objects.all().delete() + @isolate_apps('schema') + @unittest.skipIf(connection.vendor == 'sqlite', 'SQLite naively remakes the table on field alteration.') + @skipUnlessDBFeature('supports_foreign_keys') + def test_unique_no_unnecessary_fk_drops(self): + """ + If AlterField isn't selective about dropping foreign key constraints + when modifying a field with a unique constraint, the AlterField + incorrectly drops and recreates the Book.author foreign key even though + it doesn't restrict the field being changed (#29193). + """ + class Author(Model): + name = CharField(max_length=254, unique=True) + + class Meta: + app_label = 'schema' + + class Book(Model): + author = ForeignKey(Author, CASCADE) + + class Meta: + app_label = 'schema' + + with connection.schema_editor() as editor: + editor.create_model(Author) + editor.create_model(Book) + new_field = CharField(max_length=255, unique=True) + new_field.model = Author + new_field.set_attributes_from_name('name') + with patch_logger('django.db.backends.schema', 'debug') as logger_calls: + with connection.schema_editor() as editor: + editor.alter_field(Author, Author._meta.get_field('name'), new_field) + # One SQL statement is executed to alter the field. + self.assertEqual(len(logger_calls), 1) + @isolate_apps('schema') @unittest.skipIf(connection.vendor == 'sqlite', 'SQLite remakes the table on field alteration.') def test_unique_and_reverse_m2m(self):