[2.0.x] 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 inc3e0adcad8
. Backport ofee17bb8a67
from master
This commit is contained in:
parent
95e1191690
commit
d5018abf1c
1
AUTHORS
1
AUTHORS
|
@ -377,6 +377,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>
|
||||
|
|
|
@ -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))
|
||||
)
|
||||
|
||||
|
||||
|
@ -735,7 +751,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']:
|
||||
|
|
|
@ -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`).
|
||||
|
|
|
@ -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`).
|
||||
|
|
|
@ -1538,6 +1538,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):
|
||||
|
|
Loading…
Reference in New Issue