From 663e48947ff8ef3e6a6275bda2d1ee1b0de13be3 Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Fri, 7 Apr 2017 15:54:40 +0200 Subject: [PATCH] Fixed #28052 -- Prevented dropping Meta.indexes when changing db_index to False. Thanks Marc Tamlyn for the report and Ian Foote/Tim Graham for review. --- django/db/backends/base/schema.py | 15 ++++++++-- docs/releases/1.11.1.txt | 3 ++ tests/schema/models.py | 7 +++++ tests/schema/tests.py | 49 ++++++++++++++++++++++++++++--- 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index ed3fb07d86..f688fd4672 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -3,6 +3,7 @@ import logging from datetime import datetime from django.db.backends.utils import strip_quotes +from django.db.models import Index from django.db.transaction import TransactionManagementError, atomic from django.utils import timezone from django.utils.encoding import force_bytes @@ -540,8 +541,16 @@ class BaseDatabaseSchemaEditor: # True | False | True | True if old_field.db_index and not old_field.unique and (not new_field.db_index or new_field.unique): # Find the index for this field - index_names = self._constraint_names(model, [old_field.column], index=True) + meta_index_names = {index.name for index in model._meta.indexes} + # Retrieve only BTREE indexes since this is what's created with + # db_index=True. + index_names = self._constraint_names(model, [old_field.column], index=True, type_=Index.suffix) for index_name in index_names: + if index_name in meta_index_names: + # There only way to check if an index was created with + # db_index=True or with Index(['field'], name='foo') + # is to look at its name (refs #28053). + continue self.execute(self._delete_constraint_sql(self.sql_delete_index, model, index_name)) # Change check constraints? if old_db_params['check'] != new_db_params['check'] and old_db_params['check']: @@ -942,7 +951,7 @@ class BaseDatabaseSchemaEditor: def _constraint_names(self, model, column_names=None, unique=None, primary_key=None, index=None, foreign_key=None, - check=None): + check=None, type_=None): """Return all constraint names matching the columns and conditions.""" if column_names is not None: column_names = [ @@ -964,5 +973,7 @@ class BaseDatabaseSchemaEditor: continue if foreign_key is not None and not infodict['foreign_key']: continue + if type_ is not None and infodict['type'] != type_: + continue result.append(name) return result diff --git a/docs/releases/1.11.1.txt b/docs/releases/1.11.1.txt index b396ec70ae..dd4f4f1fd2 100644 --- a/docs/releases/1.11.1.txt +++ b/docs/releases/1.11.1.txt @@ -72,3 +72,6 @@ Bugfixes * Prevented ``AddIndex`` and ``RemoveIndex`` from mutating model state (:ticket:`28043`). + +* Prevented migrations from dropping database indexes from ``Meta.indexes`` + when changing ``Field.db_index`` to ``False`` (:ticket:`28052`). diff --git a/tests/schema/models.py b/tests/schema/models.py index e8a5f1c20f..0c73760d96 100644 --- a/tests/schema/models.py +++ b/tests/schema/models.py @@ -33,6 +33,13 @@ class AuthorWithEvenLongerName(models.Model): apps = new_apps +class AuthorWithIndexedName(models.Model): + name = models.CharField(max_length=255, db_index=True) + + class Meta: + apps = new_apps + + class Book(models.Model): author = models.ForeignKey(Author, models.CASCADE) title = models.CharField(max_length=100, db_index=True) diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 833611df3a..0bda5c990b 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -29,10 +29,11 @@ from .fields import ( CustomManyToManyField, InheritedManyToManyField, MediumBlobField, ) from .models import ( - Author, AuthorWithDefaultHeight, AuthorWithEvenLongerName, Book, - BookForeignObj, BookWeak, BookWithLongName, BookWithO2O, BookWithoutAuthor, - BookWithSlug, IntegerPK, Node, Note, NoteRename, Tag, TagIndexed, - TagM2MTest, TagUniqueRename, Thing, UniqueTest, new_apps, + Author, AuthorWithDefaultHeight, AuthorWithEvenLongerName, + AuthorWithIndexedName, Book, BookForeignObj, BookWeak, BookWithLongName, + BookWithO2O, BookWithoutAuthor, BookWithSlug, IntegerPK, Node, Note, + NoteRename, Tag, TagIndexed, TagM2MTest, TagUniqueRename, Thing, + UniqueTest, new_apps, ) @@ -1633,6 +1634,46 @@ class SchemaTests(TransactionTestCase): editor.remove_index(Author, index) self.assertNotIn('name', self.get_indexes(Author._meta.db_table)) + def test_remove_db_index_doesnt_remove_custom_indexes(self): + """ + Changing db_index to False doesn't remove indexes from Meta.indexes. + """ + with connection.schema_editor() as editor: + editor.create_model(AuthorWithIndexedName) + # Ensure the table has its index + self.assertIn('name', self.get_indexes(AuthorWithIndexedName._meta.db_table)) + + # Add the custom index + index = Index(fields=['-name'], name='author_name_idx') + author_index_name = index.name + with connection.schema_editor() as editor: + db_index_name = editor._create_index_name( + model=AuthorWithIndexedName, + column_names=('name',), + ) + if connection.features.uppercases_column_names: + author_index_name = author_index_name.upper() + db_index_name = db_index_name.upper() + try: + AuthorWithIndexedName._meta.indexes = [index] + with connection.schema_editor() as editor: + editor.add_index(AuthorWithIndexedName, index) + old_constraints = self.get_constraints(AuthorWithIndexedName._meta.db_table) + self.assertIn(author_index_name, old_constraints) + self.assertIn(db_index_name, old_constraints) + # Change name field to db_index=False + old_field = AuthorWithIndexedName._meta.get_field('name') + new_field = CharField(max_length=255) + new_field.set_attributes_from_name('name') + with connection.schema_editor() as editor: + editor.alter_field(AuthorWithIndexedName, old_field, new_field, strict=True) + new_constraints = self.get_constraints(AuthorWithIndexedName._meta.db_table) + self.assertNotIn(db_index_name, new_constraints) + # The index from Meta.indexes is still in the database. + self.assertIn(author_index_name, new_constraints) + finally: + AuthorWithIndexedName._meta.indexes = [] + def test_order_index(self): """ Indexes defined with ordering (ASC/DESC) defined on column