Fixed #29193 -- Prevented unnecessary foreign key drops when altering a unique field.
Stopped dropping and recreating foreign key constraints on other fields
in the same table as the one which is actually being altered in an
AlterField operation.
Regression in c3e0adcad8
.
This commit is contained in:
parent
003334f8af
commit
ee17bb8a67
1
AUTHORS
1
AUTHORS
|
@ -382,6 +382,7 @@ answer newbie questions, and generally made Django that much better:
|
||||||
Jensen Cochran <jensen.cochran@gmail.com>
|
Jensen Cochran <jensen.cochran@gmail.com>
|
||||||
Jeong-Min Lee <falsetru@gmail.com>
|
Jeong-Min Lee <falsetru@gmail.com>
|
||||||
Jérémie Blaser <blaserje@gmail.com>
|
Jérémie Blaser <blaserje@gmail.com>
|
||||||
|
Jeremy Bowman <https://github.com/jmbowman>
|
||||||
Jeremy Carbaugh <jcarbaugh@gmail.com>
|
Jeremy Carbaugh <jcarbaugh@gmail.com>
|
||||||
Jeremy Dunck <jdunck@gmail.com>
|
Jeremy Dunck <jdunck@gmail.com>
|
||||||
Jeremy Lainé <jeremy.laine@m4x.org>
|
Jeremy Lainé <jeremy.laine@m4x.org>
|
||||||
|
|
|
@ -14,12 +14,28 @@ from django.utils.encoding import force_bytes
|
||||||
logger = logging.getLogger('django.db.backends.schema')
|
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):
|
def _related_non_m2m_objects(old_field, new_field):
|
||||||
# Filter out m2m objects from reverse relations.
|
# Filter out m2m objects from reverse relations.
|
||||||
# Return (old_relation, new_relation) tuples.
|
# Return (old_relation, new_relation) tuples.
|
||||||
return zip(
|
return zip(
|
||||||
(obj for obj in old_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 not obj.field.many_to_many)
|
(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
|
# Rebuild FKs that pointed to us if we previously had to drop them
|
||||||
if drop_foreign_keys:
|
if drop_foreign_keys:
|
||||||
for rel in new_field.model._meta.related_objects:
|
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"))
|
self.execute(self._create_fk_sql(rel.related_model, rel.field, "_fk"))
|
||||||
# Does it have check constraints we need to add?
|
# Does it have check constraints we need to add?
|
||||||
if old_db_params['check'] != new_db_params['check'] and new_db_params['check']:
|
if old_db_params['check'] != new_db_params['check'] and new_db_params['check']:
|
||||||
|
|
|
@ -9,4 +9,6 @@ Django 1.11.13 fixes several bugs in 1.11.12.
|
||||||
Bugfixes
|
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`).
|
||||||
|
|
|
@ -11,3 +11,7 @@ Bugfixes
|
||||||
|
|
||||||
* Corrected the import paths that ``inspectdb`` generates for
|
* Corrected the import paths that ``inspectdb`` generates for
|
||||||
``django.contrib.postgres`` fields (:ticket:`29307`).
|
``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`).
|
||||||
|
|
|
@ -1545,6 +1545,40 @@ class SchemaTests(TransactionTestCase):
|
||||||
TagUniqueRename.objects.create(title="bar", slug2="foo")
|
TagUniqueRename.objects.create(title="bar", slug2="foo")
|
||||||
Tag.objects.all().delete()
|
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')
|
@isolate_apps('schema')
|
||||||
@unittest.skipIf(connection.vendor == 'sqlite', 'SQLite remakes the table on field alteration.')
|
@unittest.skipIf(connection.vendor == 'sqlite', 'SQLite remakes the table on field alteration.')
|
||||||
def test_unique_and_reverse_m2m(self):
|
def test_unique_and_reverse_m2m(self):
|
||||||
|
|
Loading…
Reference in New Issue