diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index b936b61fb8..39a7e5e623 100644 --- a/django/db/backends/base/schema.py +++ b/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): diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index 1a9fb1523a..36adb22584 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/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) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 3c82473fe1..a0146710e6 100644 --- a/django/db/migrations/autodetector.py +++ b/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) diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index 12ec9ffdb8..32cfc96afb 100644 --- a/django/db/migrations/state.py +++ b/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): diff --git a/django/db/models/base.py b/django/db/models/base.py index 80d98b1399..e15ca00609 100644 --- a/django/db/models/base.py +++ b/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 diff --git a/django/db/models/indexes.py b/django/db/models/indexes.py index 694d904a87..e3f94f0bba 100644 --- a/django/db/models/indexes.py +++ b/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): diff --git a/django/db/models/options.py b/django/db/models/options.py index 19dce58663..670e2a5a2f 100644 --- a/django/db/models/options.py +++ b/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 diff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt index 2de2d1a144..739b2bcdf8 100644 --- a/docs/ref/migration-operations.txt +++ b/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`` --------------- diff --git a/docs/ref/models/indexes.txt b/docs/ref/models/indexes.txt index c3c8d1b6eb..b63e86517c 100644 --- a/docs/ref/models/indexes.txt +++ b/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 ` 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. diff --git a/docs/ref/models/options.txt b/docs/ref/models/options.txt index 6569c774e8..5d385adf24 100644 --- a/docs/ref/models/options.txt +++ b/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 ` 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`` ------------------- diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 7265330057..e1469760ee 100644 --- a/tests/migrations/test_autodetector.py +++ b/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]) diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 86e58cc3e6..fdb956ae26 100644 --- a/tests/migrations/test_operations.py +++ b/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. diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index e955f1c63f..9428ea060d 100644 --- a/tests/migrations/test_state.py +++ b/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. 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. diff --git a/tests/model_indexes/tests.py b/tests/model_indexes/tests.py index e979a00de7..b3f5d04dc1 100644 --- a/tests/model_indexes/tests.py +++ b/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'})