Fixed #26808 -- Added Meta.indexes for class-based indexes.

* Added the index name to its deconstruction.
* Added indexes to sqlite3.schema._remake_table() so that indexes
  aren't dropped when _remake_table() is called.

Thanks timgraham & MarkusH for review and advice.

django/db/backends/base/schema.py

@@ -889,8 +889,8 @@ class BaseDatabaseSchemaEditor(object):
 
     def _model_indexes_sql(self, model):
         """
-        Return all index SQL statements (field indexes, index_together) for the
-        specified model, as a list.
+        Return all index SQL statements (field indexes, index_together,
+        Meta.indexes) for the specified model, as a list.
         """
         if not model._meta.managed or model._meta.proxy or model._meta.swapped:
             return []
@@ -901,6 +901,9 @@ class BaseDatabaseSchemaEditor(object):
         for field_names in model._meta.index_together:
             fields = [model._meta.get_field(field) for field in field_names]
             output.append(self._create_index_sql(model, fields, suffix="_idx"))
+
+        for index in model._meta.indexes:
+            output.append(index.create_sql(model, self))
         return output
 
     def _field_indexes_sql(self, model, field):

django/db/backends/sqlite3/schema.py

@@ -156,12 +156,20 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             for index in model._meta.index_together
         ]
 
+        indexes = model._meta.indexes
+        if delete_field:
+            indexes = [
+                index for index in indexes
+                if delete_field.name not in index.fields
+            ]
+
         # Construct a new model for the new state
         meta_contents = {
             'app_label': model._meta.app_label,
             'db_table': model._meta.db_table,
             'unique_together': unique_together,
             'index_together': index_together,
+            'indexes': indexes,
             'apps': apps,
         }
         meta = type("Meta", tuple(), meta_contents)

django/db/migrations/autodetector.py

@@ -126,6 +126,7 @@ class MigrationAutodetector(object):
         # We'll then go through that list later and order it and split
         # into migrations to resolve dependencies caused by M2Ms and FKs.
         self.generated_operations = {}
+        self.altered_indexes = {}
 
         # Prepare some old/new state and model lists, separating
         # proxy models and ignoring unmigrated apps.
@@ -175,6 +176,12 @@ class MigrationAutodetector(object):
         self.generate_altered_options()
         self.generate_altered_managers()
 
+        # Create the altered indexes and store them in self.altered_indexes.
+        # This avoids the same computation in generate_removed_indexes()
+        # and generate_added_indexes().
+        self.create_altered_indexes()
+        # Generate index removal operations before field is removed
+        self.generate_removed_indexes()
         # Generate field operations
         self.generate_renamed_fields()
         self.generate_removed_fields()
@@ -182,6 +189,7 @@ class MigrationAutodetector(object):
         self.generate_altered_fields()
         self.generate_altered_unique_together()
         self.generate_altered_index_together()
+        self.generate_added_indexes()
         self.generate_altered_db_table()
         self.generate_altered_order_with_respect_to()
 
@@ -521,6 +529,9 @@ class MigrationAutodetector(object):
                     related_fields[field.name] = field
                 if getattr(field.remote_field, "through", None) and not field.remote_field.through._meta.auto_created:
                     related_fields[field.name] = field
+            # Are there any indexes to defer?
+            indexes = model_state.options['indexes']
+            model_state.options['indexes'] = []
             # Are there unique/index_together to defer?
             unique_together = model_state.options.pop('unique_together', None)
             index_together = model_state.options.pop('index_together', None)
@@ -581,6 +592,15 @@ class MigrationAutodetector(object):
                 for name, field in sorted(related_fields.items())
             ]
             related_dependencies.append((app_label, model_name, None, True))
+            for index in indexes:
+                self.add_operation(
+                    app_label,
+                    operations.AddIndex(
+                        model_name=model_name,
+                        index=index,
+                    ),
+                    dependencies=related_dependencies,
+                )
             if unique_together:
                 self.add_operation(
                     app_label,
@@ -919,6 +939,46 @@ class MigrationAutodetector(object):
                     self._generate_removed_field(app_label, model_name, field_name)
                     self._generate_added_field(app_label, model_name, field_name)
 
+    def create_altered_indexes(self):
+        option_name = operations.AddIndex.option_name
+        for app_label, model_name in sorted(self.kept_model_keys):
+            old_model_name = self.renamed_models.get((app_label, model_name), model_name)
+            old_model_state = self.from_state.models[app_label, old_model_name]
+            new_model_state = self.to_state.models[app_label, model_name]
+
+            old_indexes = old_model_state.options[option_name]
+            new_indexes = new_model_state.options[option_name]
+            add_idx = [idx for idx in new_indexes if idx not in old_indexes]
+            rem_idx = [idx for idx in old_indexes if idx not in new_indexes]
+
+            self.altered_indexes.update({
+                (app_label, model_name): {
+                    'added_indexes': add_idx, 'removed_indexes': rem_idx,
+                }
+            })
+
+    def generate_added_indexes(self):
+        for (app_label, model_name), alt_indexes in self.altered_indexes.items():
+            for index in alt_indexes['added_indexes']:
+                self.add_operation(
+                    app_label,
+                    operations.AddIndex(
+                        model_name=model_name,
+                        index=index,
+                    )
+                )
+
+    def generate_removed_indexes(self):
+        for (app_label, model_name), alt_indexes in self.altered_indexes.items():
+            for index in alt_indexes['removed_indexes']:
+                self.add_operation(
+                    app_label,
+                    operations.RemoveIndex(
+                        model_name=model_name,
+                        name=index.name,
+                    )
+                )
+
     def _get_dependencies_for_foreign_key(self, field):
         # Account for FKs to swappable models
         swappable_setting = getattr(field, 'swappable_setting', None)

django/db/migrations/state.py

@@ -353,6 +353,13 @@ class ModelState(object):
                     'ModelState.fields cannot refer to a model class - "%s.through" does. '
                     'Use a string reference instead.' % name
                 )
+        # Sanity-check that indexes have their name set.
+        for index in self.options['indexes']:
+            if not index.name:
+                raise ValueError(
+                    "Indexes passed to ModelState require a name attribute. "
+                    "%r doesn't have one." % index
+                )
 
     @cached_property
     def name_lower(self):

django/db/models/base.py

@@ -298,6 +298,13 @@ class ModelBase(type):
                 else:
                     new_class.add_to_class(field.name, copy.deepcopy(field))
 
+        # Set the name of _meta.indexes. This can't be done in
+        # Options.contribute_to_class() because fields haven't been added to
+        # the model at that point.
+        for index in new_class._meta.indexes:
+            if not index.name:
+                index.set_name_with_model(new_class)
+
         if abstract:
             # Abstract base models can't be instantiated and don't appear in
             # the list of models for an app. We do the final setup for them a

django/db/models/indexes.py

@@ -60,7 +60,7 @@ class Index(object):
     def deconstruct(self):
         path = '%s.%s' % (self.__class__.__module__, self.__class__.__name__)
         path = path.replace('django.db.models.indexes', 'django.db.models')
-        return (path, (), {'fields': self.fields})
+        return (path, (), {'fields': self.fields, 'name': self.name})
 
     @staticmethod
     def _hash_generator(*args):

django/db/models/options.py

@@ -99,6 +99,7 @@ class Options(object):
         self.db_table = ''
         self.ordering = []
         self._ordering_clash = False
+        self.indexes = []
         self.unique_together = []
         self.index_together = []
         self.select_on_save = False

docs/ref/migration-operations.txt

@@ -202,22 +202,6 @@ is set, its column name).
 Creates an index in the database table for the model with ``model_name``.
 ``index`` is an instance of the :class:`~django.db.models.Index` class.
 
-For example, to add an index on the ``title`` and ``author`` fields of the
-``Book`` model::
-
-    from django.db import migrations, models
-
-    class Migration(migrations.Migration):
-        operations = [
-            migrations.AddIndex(
-                'Book',
-                models.Index(fields=['title', 'author'], name='my_index_name'),
-            ),
-        ]
-
-If you're writing your own migration to add an index, you must assign a
-``name`` to the ``index`` as done above.
-
 ``RemoveIndex``
 ---------------
 

docs/ref/models/indexes.txt

@@ -8,8 +8,10 @@ Model index reference
 
 .. versionadded:: 1.11
 
-Index classes ease creating database indexes. This document explains the API
-references of :class:`Index` which includes the `index options`_.
+Index classes ease creating database indexes. They can be added using the
+:attr:`Meta.indexes <django.db.models.Options.indexes>` option. This document
+explains the API references of :class:`Index` which includes the `index
+options`_.
 
 .. admonition:: Referencing built-in indexes
 
@@ -40,9 +42,3 @@ A list of the name of the fields on which the index is desired.
 The name of the index. If ``name`` isn't provided Django will auto-generate a
 name. For compatibility with different databases, index names cannot be longer
 than 30 characters and shouldn't start with a number (0-9) or underscore (_).
-
-.. seealso::
-
-    Use the :class:`~django.db.migrations.operations.AddIndex` and
-    :class:`~django.db.migrations.operations.RemoveIndex` operations to add
-    and remove indexes.

docs/ref/models/options.txt

@@ -390,6 +390,28 @@ Django quotes column and table names behind the scenes.
     See :meth:`django.db.models.Model.save()` for more about the old and
     new saving algorithm.
 
+``indexes``
+-----------
+
+.. attribute:: Options.indexes
+
+    .. versionadded:: 1.11
+
+    A list of :doc:`indexes </ref/models/indexes>` that you want to define on
+    the model::
+
+        from django.db import models
+
+        class Customer(models.Model):
+            first_name = models.CharField(max_length=100)
+            last_name = models.CharField(max_length=100)
+
+            class Meta:
+                indexes = [
+                    models.Index(fields=['last_name', 'first_name']),
+                    models.Index(fields=['first_name'], name='first_name_idx'),
+                ]
+
 ``unique_together``
 -------------------
 

tests/migrations/test_autodetector.py

@@ -370,6 +370,20 @@ class AutodetectorTests(TestCase):
         ("authors", models.ManyToManyField("testapp.Author", through="otherapp.Attribution")),
         ("title", models.CharField(max_length=200)),
     ])
+    book_indexes = ModelState("otherapp", "Book", [
+        ("id", models.AutoField(primary_key=True)),
+        ("author", models.ForeignKey("testapp.Author", models.CASCADE)),
+        ("title", models.CharField(max_length=200)),
+    ], {
+        "indexes": [models.Index(fields=["author", "title"], name="book_title_author_idx")],
+    })
+    book_unordered_indexes = ModelState("otherapp", "Book", [
+        ("id", models.AutoField(primary_key=True)),
+        ("author", models.ForeignKey("testapp.Author", models.CASCADE)),
+        ("title", models.CharField(max_length=200)),
+    ], {
+        "indexes": [models.Index(fields=["title", "author"], name="book_author_title_idx")],
+    })
     book_foo_together = ModelState("otherapp", "Book", [
         ("id", models.AutoField(primary_key=True)),
         ("author", models.ForeignKey("testapp.Author", models.CASCADE)),
@@ -432,7 +446,10 @@ class AutodetectorTests(TestCase):
         ("id", models.AutoField(primary_key=True)),
         ("knight", models.ForeignKey("eggs.Knight", models.CASCADE)),
         ("parent", models.ForeignKey("eggs.Rabbit", models.CASCADE)),
-    ], {"unique_together": {("parent", "knight")}})
+    ], {
+        "unique_together": {("parent", "knight")},
+        "indexes": [models.Index(fields=["parent", "knight"], name='rabbit_circular_fk_index')],
+    })
 
     def repr_changes(self, changes, include_dependencies=False):
         output = ""
@@ -978,16 +995,18 @@ class AutodetectorTests(TestCase):
         self.assertOperationAttributes(changes, "testapp", 0, 2, name="publisher")
         self.assertMigrationDependencies(changes, 'testapp', 0, [])
 
-    def test_same_app_circular_fk_dependency_and_unique_together(self):
+    def test_same_app_circular_fk_dependency_with_unique_together_and_indexes(self):
         """
         #22275 - Tests that a migration with circular FK dependency does not try
-        to create unique together constraint before creating all required fields
-        first.
+        to create unique together constraint and indexes before creating all
+        required fields first.
         """
         changes = self.get_changes([], [self.knight, self.rabbit])
         # Right number/type of migrations?
         self.assertNumberMigrations(changes, 'eggs', 1)
-        self.assertOperationTypes(changes, 'eggs', 0, ["CreateModel", "CreateModel", "AlterUniqueTogether"])
+        self.assertOperationTypes(
+            changes, 'eggs', 0, ["CreateModel", "CreateModel", "AddIndex", "AlterUniqueTogether"]
+        )
         self.assertNotIn("unique_together", changes['eggs'][0].operations[0].options)
         self.assertNotIn("unique_together", changes['eggs'][0].operations[1].options)
         self.assertMigrationDependencies(changes, 'eggs', 0, [])
@@ -1135,6 +1154,51 @@ class AutodetectorTests(TestCase):
         for t in tests:
             test(*t)
 
+    def test_create_model_with_indexes(self):
+        """Test creation of new model with indexes already defined."""
+        author = ModelState('otherapp', 'Author', [
+            ('id', models.AutoField(primary_key=True)),
+            ('name', models.CharField(max_length=200)),
+        ], {'indexes': [models.Index(fields=['name'], name='create_model_with_indexes_idx')]})
+        changes = self.get_changes([], [author])
+        added_index = models.Index(fields=['name'], name='create_model_with_indexes_idx')
+        # Right number of migrations?
+        self.assertEqual(len(changes['otherapp']), 1)
+        # Right number of actions?
+        migration = changes['otherapp'][0]
+        self.assertEqual(len(migration.operations), 2)
+        # Right actions order?
+        self.assertOperationTypes(changes, 'otherapp', 0, ['CreateModel', 'AddIndex'])
+        self.assertOperationAttributes(changes, 'otherapp', 0, 0, name='Author')
+        self.assertOperationAttributes(changes, 'otherapp', 0, 1, model_name='author', index=added_index)
+
+    def test_add_indexes(self):
+        """Test change detection of new indexes."""
+        changes = self.get_changes([self.author_empty, self.book], [self.author_empty, self.book_indexes])
+        self.assertNumberMigrations(changes, 'otherapp', 1)
+        self.assertOperationTypes(changes, 'otherapp', 0, ['AddIndex'])
+        added_index = models.Index(fields=['author', 'title'], name='book_title_author_idx')
+        self.assertOperationAttributes(changes, 'otherapp', 0, 0, model_name='book', index=added_index)
+
+    def test_remove_indexes(self):
+        """Test change detection of removed indexes."""
+        changes = self.get_changes([self.author_empty, self.book_indexes], [self.author_empty, self.book])
+        # Right number/type of migrations?
+        self.assertNumberMigrations(changes, 'otherapp', 1)
+        self.assertOperationTypes(changes, 'otherapp', 0, ['RemoveIndex'])
+        self.assertOperationAttributes(changes, 'otherapp', 0, 0, model_name='book', name='book_title_author_idx')
+
+    def test_order_fields_indexes(self):
+        """Test change detection of reordering of fields in indexes."""
+        changes = self.get_changes(
+            [self.author_empty, self.book_indexes], [self.author_empty, self.book_unordered_indexes]
+        )
+        self.assertNumberMigrations(changes, 'otherapp', 1)
+        self.assertOperationTypes(changes, 'otherapp', 0, ['RemoveIndex', 'AddIndex'])
+        self.assertOperationAttributes(changes, 'otherapp', 0, 0, model_name='book', name='book_title_author_idx')
+        added_index = models.Index(fields=['title', 'author'], name='book_author_title_idx')
+        self.assertOperationAttributes(changes, 'otherapp', 0, 1, model_name='book', index=added_index)
+
     def test_add_foo_together(self):
         """Tests index/unique_together detection."""
         changes = self.get_changes([self.author_empty, self.book], [self.author_empty, self.book_foo_together])

tests/migrations/test_operations.py

@@ -51,7 +51,7 @@ class OperationTestBase(MigrationTestBase):
         return project_state, new_state
 
     def set_up_test_model(
-            self, app_label, second_model=False, third_model=False, multicol_index=False,
+            self, app_label, second_model=False, third_model=False, index=False, multicol_index=False,
             related_model=False, mti_model=False, proxy_model=False, manager_model=False,
             unique_together=False, options=False, db_table=None, index_together=False):
         """
@@ -96,6 +96,11 @@ class OperationTestBase(MigrationTestBase):
             ],
             options=model_options,
         )]
+        if index:
+            operations.append(migrations.AddIndex(
+                "Pony",
+                models.Index(fields=["pink"], name="pony_pink_idx")
+            ))
         if multicol_index:
             operations.append(migrations.AddIndex(
                 "Pony",
@@ -1447,6 +1452,43 @@ class OperationTests(OperationTestBase):
         self.assertEqual(definition[1], [])
         self.assertEqual(definition[2], {'model_name': "Pony", 'name': "pony_test_idx"})
 
+        # Also test a field dropped with index - sqlite remake issue
+        operations = [
+            migrations.RemoveIndex("Pony", "pony_test_idx"),
+            migrations.RemoveField("Pony", "pink"),
+        ]
+        self.assertColumnExists("test_rmin_pony", "pink")
+        self.assertIndexExists("test_rmin_pony", ["pink", "weight"])
+        # Test database alteration
+        new_state = project_state.clone()
+        self.apply_operations('test_rmin', new_state, operations=operations)
+        self.assertColumnNotExists("test_rmin_pony", "pink")
+        self.assertIndexNotExists("test_rmin_pony", ["pink", "weight"])
+        # And test reversal
+        self.unapply_operations("test_rmin", project_state, operations=operations)
+        self.assertIndexExists("test_rmin_pony", ["pink", "weight"])
+
+    def test_alter_field_with_index(self):
+        """
+        Test AlterField operation with an index to ensure indexes created via
+        Meta.indexes don't get dropped with sqlite3 remake.
+        """
+        project_state = self.set_up_test_model("test_alflin", index=True)
+        operation = migrations.AlterField("Pony", "pink", models.IntegerField(null=True))
+        new_state = project_state.clone()
+        operation.state_forwards("test_alflin", new_state)
+        # Test the database alteration
+        self.assertColumnNotNull("test_alflin_pony", "pink")
+        with connection.schema_editor() as editor:
+            operation.database_forwards("test_alflin", editor, project_state, new_state)
+        # Ensure that index hasn't been dropped
+        self.assertIndexExists("test_alflin_pony", ["pink"])
+        # And test reversal
+        with connection.schema_editor() as editor:
+            operation.database_backwards("test_alflin", editor, new_state, project_state)
+        # Ensure the index is still there
+        self.assertIndexExists("test_alflin_pony", ["pink"])
+
     def test_alter_index_together(self):
         """
         Tests the AlterIndexTogether operation.

tests/migrations/test_state.py

@@ -65,6 +65,7 @@ class StateTests(SimpleTestCase):
                 apps = new_apps
                 verbose_name = "tome"
                 db_table = "test_tome"
+                indexes = [models.Index(fields=['title'])]
 
         class Food(models.Model):
 
@@ -116,6 +117,8 @@ class StateTests(SimpleTestCase):
         food_no_managers_state = project_state.models['migrations', 'foodnomanagers']
         food_no_default_manager_state = project_state.models['migrations', 'foodnodefaultmanager']
         food_order_manager_state = project_state.models['migrations', 'foodorderedmanagers']
+        book_index = models.Index(fields=['title'])
+        book_index.set_name_with_model(Book)
 
         self.assertEqual(author_state.app_label, "migrations")
         self.assertEqual(author_state.name, "Author")
@@ -135,7 +138,10 @@ class StateTests(SimpleTestCase):
         self.assertEqual(book_state.fields[1][1].max_length, 1000)
         self.assertIs(book_state.fields[2][1].null, False)
         self.assertEqual(book_state.fields[3][1].__class__.__name__, "ManyToManyField")
-        self.assertEqual(book_state.options, {"verbose_name": "tome", "db_table": "test_tome", "indexes": []})
+        self.assertEqual(
+            book_state.options,
+            {"verbose_name": "tome", "db_table": "test_tome", "indexes": [book_index]},
+        )
         self.assertEqual(book_state.bases, (models.Model, ))
 
         self.assertEqual(author_proxy_state.app_label, "migrations")
@@ -947,6 +953,13 @@ class ModelStateTests(SimpleTestCase):
         ):
             ModelState('app', 'Model', [('field', field)])
 
+    def test_sanity_index_name(self):
+        field = models.IntegerField()
+        options = {'indexes': [models.Index(fields=['field'])]}
+        msg = "Indexes passed to ModelState require a name attribute. <Index: fields='field'> doesn't have one."
+        with self.assertRaisesMessage(ValueError, msg):
+            ModelState('app', 'Model', [('field', field)], options=options)
+
     def test_fields_immutability(self):
         """
         Tests that rendering a model state doesn't alter its internal fields.

tests/model_indexes/tests.py

@@ -16,6 +16,9 @@ class IndexesTests(TestCase):
         index = models.Index(fields=['title'])
         same_index = models.Index(fields=['title'])
         another_index = models.Index(fields=['title', 'author'])
+        index.model = Book
+        same_index.model = Book
+        another_index.model = Book
         self.assertEqual(index, same_index)
         self.assertNotEqual(index, another_index)
 
@@ -56,7 +59,8 @@ class IndexesTests(TestCase):
 
     def test_deconstruction(self):
         index = models.Index(fields=['title'])
+        index.set_name_with_model(Book)
         path, args, kwargs = index.deconstruct()
         self.assertEqual(path, 'django.db.models.Index')
         self.assertEqual(args, ())
-        self.assertEqual(kwargs, {'fields': ['title']})
+        self.assertEqual(kwargs, {'fields': ['title'], 'name': 'model_index_title_196f42_idx'})