From ae635cc365581889454d2b3c278b3e3a38bd7809 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Wed, 13 May 2015 17:40:57 +0200 Subject: [PATCH] Fixed #24757 -- Recreated MySQL index when needed during combined index removal Thanks Thomas Recouvreux for the report and Tim Graham for the tests and review. --- django/db/backends/base/schema.py | 31 +++++++++++-------------- django/db/backends/mysql/schema.py | 18 +++++++++++++++ docs/releases/1.8.2.txt | 3 +++ tests/schema/tests.py | 36 ++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 18 deletions(-) diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 3308b9c7f19..af4ddfe07aa 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -315,15 +315,7 @@ class BaseDatabaseSchemaEditor(object): news = set(tuple(fields) for fields in new_unique_together) # Deleted uniques for fields in olds.difference(news): - columns = [model._meta.get_field(field).column for field in fields] - constraint_names = self._constraint_names(model, columns, unique=True) - if len(constraint_names) != 1: - raise ValueError("Found wrong number (%s) of constraints for %s(%s)" % ( - len(constraint_names), - model._meta.db_table, - ", ".join(columns), - )) - self.execute(self._delete_constraint_sql(self.sql_delete_unique, model, constraint_names[0])) + self._delete_composed_index(model, fields, {'unique': True}, self.sql_delete_unique) # Created uniques for fields in news.difference(olds): columns = [model._meta.get_field(field).column for field in fields] @@ -339,20 +331,23 @@ class BaseDatabaseSchemaEditor(object): news = set(tuple(fields) for fields in new_index_together) # Deleted indexes for fields in olds.difference(news): - columns = [model._meta.get_field(field).column for field in fields] - constraint_names = self._constraint_names(model, list(columns), index=True) - if len(constraint_names) != 1: - raise ValueError("Found wrong number (%s) of constraints for %s(%s)" % ( - len(constraint_names), - model._meta.db_table, - ", ".join(columns), - )) - self.execute(self._delete_constraint_sql(self.sql_delete_index, model, constraint_names[0])) + self._delete_composed_index(model, fields, {'index': True}, self.sql_delete_index) # Created indexes for field_names in news.difference(olds): fields = [model._meta.get_field(field) for field in field_names] self.execute(self._create_index_sql(model, fields, suffix="_idx")) + def _delete_composed_index(self, model, fields, constraint_kwargs, sql): + columns = [model._meta.get_field(field).column for field in fields] + constraint_names = self._constraint_names(model, columns, **constraint_kwargs) + if len(constraint_names) != 1: + raise ValueError("Found wrong number (%s) of constraints for %s(%s)" % ( + len(constraint_names), + model._meta.db_table, + ", ".join(columns), + )) + self.execute(self._delete_constraint_sql(sql, model, constraint_names[0])) + def alter_db_table(self, model, old_db_table, new_db_table): """ Renames the table a model points to. diff --git a/django/db/backends/mysql/schema.py b/django/db/backends/mysql/schema.py index f2f3f0a4ed8..7c409598ae1 100644 --- a/django/db/backends/mysql/schema.py +++ b/django/db/backends/mysql/schema.py @@ -62,6 +62,24 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): field.db_index = False return super(DatabaseSchemaEditor, self)._model_indexes_sql(model) + def _delete_composed_index(self, model, fields, *args): + """ + MySQL can remove an implicit FK index on a field when that field is + covered by another index like a unique_together. "covered" here means + that the more complex index starts like the simpler one. + http://bugs.mysql.com/bug.php?id=37910 / Django ticket #24757 + We check here before removing the [unique|index]_together if we have to + recreate a FK index. + """ + first_field = model._meta.get_field(fields[0]) + if first_field.get_internal_type() == 'ForeignKey': + constraint_names = self._constraint_names(model, fields[0], index=True) + if not constraint_names: + self.execute( + self._create_index_sql(model, [model._meta.get_field(fields[0])], suffix="") + ) + return super(DatabaseSchemaEditor, self)._delete_composed_index(model, fields, *args) + def _alter_column_type_sql(self, table, old_field, new_field, new_type): # Keep null property of old field, if it has changed, it will be handled separately if old_field.null: diff --git a/docs/releases/1.8.2.txt b/docs/releases/1.8.2.txt index 23483c2a5ef..f14036b51fa 100644 --- a/docs/releases/1.8.2.txt +++ b/docs/releases/1.8.2.txt @@ -27,3 +27,6 @@ Bugfixes :ticket:`24712`). * Fixed ``isnull`` lookup for ``HStoreField`` (:ticket:`24751`). + +* Fixed a MySQL crash when a migration removes a combined index (unique_together + or index_together) containing a foreign key (:ticket:`24757`). diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 6cfc30a415c..de38f6db949 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -1084,6 +1084,24 @@ class SchemaTests(TransactionTestCase): self.assertRaises(IntegrityError, UniqueTest.objects.create, year=2012, slug="foo") UniqueTest.objects.all().delete() + def test_unique_together_with_fk(self): + """ + Tests removing and adding unique_together constraints that include + a foreign key. + """ + # Create the table + with connection.schema_editor() as editor: + editor.create_model(Author) + editor.create_model(Book) + # Ensure the fields are unique to begin with + self.assertEqual(Book._meta.unique_together, ()) + # Add the unique_together constraint + with connection.schema_editor() as editor: + editor.alter_unique_together(Book, [], [['author', 'title']]) + # Alter it back + with connection.schema_editor() as editor: + editor.alter_unique_together(Book, [['author', 'title']], []) + def test_index_together(self): """ Tests removing and adding index_together constraints on a model. @@ -1127,6 +1145,24 @@ class SchemaTests(TransactionTestCase): ), ) + def test_index_together_with_fk(self): + """ + Tests removing and adding index_together constraints that include + a foreign key. + """ + # Create the table + with connection.schema_editor() as editor: + editor.create_model(Author) + editor.create_model(Book) + # Ensure the fields are unique to begin with + self.assertEqual(Book._meta.index_together, ()) + # Add the unique_together constraint + with connection.schema_editor() as editor: + editor.alter_index_together(Book, [], [['author', 'title']]) + # Alter it back + with connection.schema_editor() as editor: + editor.alter_index_together(Book, [['author', 'title']], []) + def test_create_index_together(self): """ Tests creating models with index_together already defined