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.
This commit is contained in:
Markus Holtermann 2017-04-07 15:54:40 +02:00 committed by Tim Graham
parent 63afe3a2bf
commit 663e48947f
4 changed files with 68 additions and 6 deletions

View File

@ -3,6 +3,7 @@ import logging
from datetime import datetime from datetime import datetime
from django.db.backends.utils import strip_quotes from django.db.backends.utils import strip_quotes
from django.db.models import Index
from django.db.transaction import TransactionManagementError, atomic from django.db.transaction import TransactionManagementError, atomic
from django.utils import timezone from django.utils import timezone
from django.utils.encoding import force_bytes from django.utils.encoding import force_bytes
@ -540,8 +541,16 @@ class BaseDatabaseSchemaEditor:
# True | False | True | True # True | False | True | True
if old_field.db_index and not old_field.unique and (not new_field.db_index or new_field.unique): 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 # 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: 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)) self.execute(self._delete_constraint_sql(self.sql_delete_index, model, index_name))
# Change check constraints? # Change check constraints?
if old_db_params['check'] != new_db_params['check'] and old_db_params['check']: 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, def _constraint_names(self, model, column_names=None, unique=None,
primary_key=None, index=None, foreign_key=None, primary_key=None, index=None, foreign_key=None,
check=None): check=None, type_=None):
"""Return all constraint names matching the columns and conditions.""" """Return all constraint names matching the columns and conditions."""
if column_names is not None: if column_names is not None:
column_names = [ column_names = [
@ -964,5 +973,7 @@ class BaseDatabaseSchemaEditor:
continue continue
if foreign_key is not None and not infodict['foreign_key']: if foreign_key is not None and not infodict['foreign_key']:
continue continue
if type_ is not None and infodict['type'] != type_:
continue
result.append(name) result.append(name)
return result return result

View File

@ -72,3 +72,6 @@ Bugfixes
* Prevented ``AddIndex`` and ``RemoveIndex`` from mutating model state * Prevented ``AddIndex`` and ``RemoveIndex`` from mutating model state
(:ticket:`28043`). (:ticket:`28043`).
* Prevented migrations from dropping database indexes from ``Meta.indexes``
when changing ``Field.db_index`` to ``False`` (:ticket:`28052`).

View File

@ -33,6 +33,13 @@ class AuthorWithEvenLongerName(models.Model):
apps = new_apps 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): class Book(models.Model):
author = models.ForeignKey(Author, models.CASCADE) author = models.ForeignKey(Author, models.CASCADE)
title = models.CharField(max_length=100, db_index=True) title = models.CharField(max_length=100, db_index=True)

View File

@ -29,10 +29,11 @@ from .fields import (
CustomManyToManyField, InheritedManyToManyField, MediumBlobField, CustomManyToManyField, InheritedManyToManyField, MediumBlobField,
) )
from .models import ( from .models import (
Author, AuthorWithDefaultHeight, AuthorWithEvenLongerName, Book, Author, AuthorWithDefaultHeight, AuthorWithEvenLongerName,
BookForeignObj, BookWeak, BookWithLongName, BookWithO2O, BookWithoutAuthor, AuthorWithIndexedName, Book, BookForeignObj, BookWeak, BookWithLongName,
BookWithSlug, IntegerPK, Node, Note, NoteRename, Tag, TagIndexed, BookWithO2O, BookWithoutAuthor, BookWithSlug, IntegerPK, Node, Note,
TagM2MTest, TagUniqueRename, Thing, UniqueTest, new_apps, NoteRename, Tag, TagIndexed, TagM2MTest, TagUniqueRename, Thing,
UniqueTest, new_apps,
) )
@ -1633,6 +1634,46 @@ class SchemaTests(TransactionTestCase):
editor.remove_index(Author, index) editor.remove_index(Author, index)
self.assertNotIn('name', self.get_indexes(Author._meta.db_table)) 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): def test_order_index(self):
""" """
Indexes defined with ordering (ASC/DESC) defined on column Indexes defined with ordering (ASC/DESC) defined on column