diff options
| -rw-r--r-- | AUTHORS | 1 | ||||
| -rw-r--r-- | django/db/backends/base/schema.py | 22 | ||||
| -rw-r--r-- | docs/releases/1.11.13.txt | 4 | ||||
| -rw-r--r-- | tests/schema/tests.py | 34 |
4 files changed, 57 insertions, 4 deletions
@@ -365,6 +365,7 @@ answer newbie questions, and generally made Django that much better: Jensen Cochran <jensen.cochran@gmail.com> Jeong-Min Lee <falsetru@gmail.com> Jérémie Blaser <blaserje@gmail.com> + Jeremy Bowman <https://github.com/jmbowman> Jeremy Carbaugh <jcarbaugh@gmail.com> Jeremy Dunck <jdunck@gmail.com> Jeremy Lainé <jeremy.laine@m4x.org> diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 8802476c07..efef3e0f3c 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -11,12 +11,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): # Filters out m2m objects from reverse relations. # Returns (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)) ) @@ -780,7 +796,7 @@ class BaseDatabaseSchemaEditor(object): # 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/tests/schema/tests.py b/tests/schema/tests.py index 36daccb19b..1f8f97188c 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -1393,6 +1393,40 @@ class SchemaTests(TransactionTestCase): 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): """ |