From eacd4977f6a4bb038e82796ba79a2f61bae330c6 Mon Sep 17 00:00:00 2001 From: David Wobrock Date: Mon, 2 May 2022 17:22:54 +0200 Subject: [PATCH] Refs #27064 -- Added RenameIndex migration operation. --- django/db/backends/base/features.py | 3 + django/db/backends/base/schema.py | 19 +++ django/db/backends/mysql/features.py | 6 + django/db/backends/mysql/schema.py | 1 + django/db/backends/oracle/features.py | 1 + django/db/backends/postgresql/features.py | 1 + django/db/migrations/operations/__init__.py | 2 + django/db/migrations/operations/models.py | 146 ++++++++++++++++++++ django/db/migrations/state.py | 22 +++ docs/ref/migration-operations.txt | 16 +++ docs/ref/schema-editor.txt | 9 ++ docs/releases/4.1.txt | 5 +- tests/migrations/test_operations.py | 143 +++++++++++++++++++ tests/migrations/test_optimizer.py | 38 +++++ 14 files changed, 411 insertions(+), 1 deletion(-) diff --git a/django/db/backends/base/features.py b/django/db/backends/base/features.py index a96dbe1b08..68cad9fef2 100644 --- a/django/db/backends/base/features.py +++ b/django/db/backends/base/features.py @@ -176,6 +176,9 @@ class BaseDatabaseFeatures: # Can it create foreign key constraints inline when adding columns? can_create_inline_fk = True + # Can an index be renamed? + can_rename_index = False + # Does it automatically index foreign keys? indexes_foreign_keys = True diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 468ab42490..f2ca8c8df9 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -131,6 +131,7 @@ class BaseDatabaseSchemaEditor: "CREATE UNIQUE INDEX %(name)s ON %(table)s " "(%(columns)s)%(include)s%(condition)s" ) + sql_rename_index = "ALTER INDEX %(old_name)s RENAME TO %(new_name)s" sql_delete_index = "DROP INDEX %(name)s" sql_create_pk = ( @@ -492,6 +493,16 @@ class BaseDatabaseSchemaEditor: return None self.execute(index.remove_sql(model, self)) + def rename_index(self, model, old_index, new_index): + if self.connection.features.can_rename_index: + self.execute( + self._rename_index_sql(model, old_index.name, new_index.name), + params=None, + ) + else: + self.remove_index(model, old_index) + self.add_index(model, new_index) + def add_constraint(self, model, constraint): """Add a constraint to a model.""" sql = constraint.create_sql(model, self) @@ -1361,6 +1372,14 @@ class BaseDatabaseSchemaEditor: name=self.quote_name(name), ) + def _rename_index_sql(self, model, old_name, new_name): + return Statement( + self.sql_rename_index, + table=Table(model._meta.db_table, self.quote_name), + old_name=self.quote_name(old_name), + new_name=self.quote_name(new_name), + ) + def _index_columns(self, table, columns, col_suffixes, opclasses): return Columns(table, columns, self.quote_name, col_suffixes=col_suffixes) diff --git a/django/db/backends/mysql/features.py b/django/db/backends/mysql/features.py index 7b9a90ab06..1261d9e9a6 100644 --- a/django/db/backends/mysql/features.py +++ b/django/db/backends/mysql/features.py @@ -344,3 +344,9 @@ class DatabaseFeatures(BaseDatabaseFeatures): and self._mysql_storage_engine != "MyISAM" and self.connection.mysql_version >= (8, 0, 13) ) + + @cached_property + def can_rename_index(self): + if self.connection.mysql_is_mariadb: + return self.connection.mysql_version >= (10, 5, 2) + return True diff --git a/django/db/backends/mysql/schema.py b/django/db/backends/mysql/schema.py index 562b209eef..d6d303f0f0 100644 --- a/django/db/backends/mysql/schema.py +++ b/django/db/backends/mysql/schema.py @@ -23,6 +23,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): sql_delete_fk = "ALTER TABLE %(table)s DROP FOREIGN KEY %(name)s" sql_delete_index = "DROP INDEX %(name)s ON %(table)s" + sql_rename_index = "ALTER TABLE %(table)s RENAME INDEX %(old_name)s TO %(new_name)s" sql_create_pk = ( "ALTER TABLE %(table)s ADD CONSTRAINT %(name)s PRIMARY KEY (%(columns)s)" diff --git a/django/db/backends/oracle/features.py b/django/db/backends/oracle/features.py index e0db3daa88..defd0a0ff8 100644 --- a/django/db/backends/oracle/features.py +++ b/django/db/backends/oracle/features.py @@ -60,6 +60,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): supports_ignore_conflicts = False max_query_params = 2**16 - 1 supports_partial_indexes = False + can_rename_index = True supports_slicing_ordering_in_compound = True allows_multiple_constraints_on_same_fields = False supports_boolean_expr_in_select_clause = False diff --git a/django/db/backends/postgresql/features.py b/django/db/backends/postgresql/features.py index 57688642eb..ce01e88603 100644 --- a/django/db/backends/postgresql/features.py +++ b/django/db/backends/postgresql/features.py @@ -60,6 +60,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): supports_update_conflicts = True supports_update_conflicts_with_target = True supports_covering_indexes = True + can_rename_index = True test_collations = { "non_default": "sv-x-icu", "swedish_ci": "sv-x-icu", diff --git a/django/db/migrations/operations/__init__.py b/django/db/migrations/operations/__init__.py index 793969ed12..987c7c1fe6 100644 --- a/django/db/migrations/operations/__init__.py +++ b/django/db/migrations/operations/__init__.py @@ -12,6 +12,7 @@ from .models import ( DeleteModel, RemoveConstraint, RemoveIndex, + RenameIndex, RenameModel, ) from .special import RunPython, RunSQL, SeparateDatabaseAndState @@ -26,6 +27,7 @@ __all__ = [ "AlterModelOptions", "AddIndex", "RemoveIndex", + "RenameIndex", "AddField", "RemoveField", "AlterField", diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 8a5c04393b..d17232e4ec 100644 --- a/django/db/migrations/operations/models.py +++ b/django/db/migrations/operations/models.py @@ -876,6 +876,152 @@ class RemoveIndex(IndexOperation): return "remove_%s_%s" % (self.model_name_lower, self.name.lower()) +class RenameIndex(IndexOperation): + """Rename an index.""" + + def __init__(self, model_name, new_name, old_name=None, old_fields=None): + if not old_name and not old_fields: + raise ValueError( + "RenameIndex requires one of old_name and old_fields arguments to be " + "set." + ) + if old_name and old_fields: + raise ValueError( + "RenameIndex.old_name and old_fields are mutually exclusive." + ) + self.model_name = model_name + self.new_name = new_name + self.old_name = old_name + self.old_fields = old_fields + + @cached_property + def old_name_lower(self): + return self.old_name.lower() + + @cached_property + def new_name_lower(self): + return self.new_name.lower() + + def deconstruct(self): + kwargs = { + "model_name": self.model_name, + "new_name": self.new_name, + } + if self.old_name: + kwargs["old_name"] = self.old_name + if self.old_fields: + kwargs["old_fields"] = self.old_fields + return (self.__class__.__qualname__, [], kwargs) + + def state_forwards(self, app_label, state): + if self.old_fields: + state.add_index( + app_label, + self.model_name_lower, + models.Index(fields=self.old_fields, name=self.new_name), + ) + state.remove_model_options( + app_label, + self.model_name_lower, + AlterIndexTogether.option_name, + self.old_fields, + ) + else: + state.rename_index( + app_label, self.model_name_lower, self.old_name, self.new_name + ) + + def database_forwards(self, app_label, schema_editor, from_state, to_state): + model = to_state.apps.get_model(app_label, self.model_name) + if not self.allow_migrate_model(schema_editor.connection.alias, model): + return + + if self.old_fields: + from_model = from_state.apps.get_model(app_label, self.model_name) + columns = [ + from_model._meta.get_field(field).column for field in self.old_fields + ] + matching_index_name = schema_editor._constraint_names( + from_model, column_names=columns, index=True + ) + if len(matching_index_name) != 1: + raise ValueError( + "Found wrong number (%s) of indexes for %s(%s)." + % ( + len(matching_index_name), + from_model._meta.db_table, + ", ".join(columns), + ) + ) + old_index = models.Index( + fields=self.old_fields, + name=matching_index_name[0], + ) + else: + from_model_state = from_state.models[app_label, self.model_name_lower] + old_index = from_model_state.get_index_by_name(self.old_name) + + to_model_state = to_state.models[app_label, self.model_name_lower] + new_index = to_model_state.get_index_by_name(self.new_name) + schema_editor.rename_index(model, old_index, new_index) + + def database_backwards(self, app_label, schema_editor, from_state, to_state): + if self.old_fields: + # Backward operation with unnamed index is a no-op. + return + + self.new_name_lower, self.old_name_lower = ( + self.old_name_lower, + self.new_name_lower, + ) + self.new_name, self.old_name = self.old_name, self.new_name + + self.database_forwards(app_label, schema_editor, from_state, to_state) + + self.new_name_lower, self.old_name_lower = ( + self.old_name_lower, + self.new_name_lower, + ) + self.new_name, self.old_name = self.old_name, self.new_name + + def describe(self): + if self.old_name: + return ( + f"Rename index {self.old_name} on {self.model_name} to {self.new_name}" + ) + return ( + f"Rename unnamed index for {self.old_fields} on {self.model_name} to " + f"{self.new_name}" + ) + + @property + def migration_name_fragment(self): + if self.old_name: + return "rename_%s_%s" % (self.old_name_lower, self.new_name_lower) + return "rename_%s_%s_%s" % ( + self.model_name_lower, + "_".join(self.old_fields), + self.new_name_lower, + ) + + def reduce(self, operation, app_label): + if ( + isinstance(operation, RenameIndex) + and self.model_name_lower == operation.model_name_lower + and operation.old_name + and self.new_name_lower == operation.old_name_lower + ): + return [ + RenameIndex( + self.model_name, + new_name=operation.new_name, + old_name=self.old_name, + old_fields=self.old_fields, + ) + ] + return super().reduce(operation, app_label) + + class AddConstraint(IndexOperation): option_name = "constraints" diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index 7d7a9174f0..ff5d0e93a9 100644 --- a/django/db/migrations/state.py +++ b/django/db/migrations/state.py @@ -187,6 +187,14 @@ class ProjectState: model_state.options.pop(key, False) self.reload_model(app_label, model_name, delay=True) + def remove_model_options(self, app_label, model_name, option_name, value_to_remove): + model_state = self.models[app_label, model_name] + if objs := model_state.options.get(option_name): + model_state.options[option_name] = [ + obj for obj in objs if tuple(obj) != tuple(value_to_remove) + ] + self.reload_model(app_label, model_name, delay=True) + def alter_model_managers(self, app_label, model_name, managers): model_state = self.models[app_label, model_name] model_state.managers = list(managers) @@ -209,6 +217,20 @@ class ProjectState: def remove_index(self, app_label, model_name, index_name): self._remove_option(app_label, model_name, "indexes", index_name) + def rename_index(self, app_label, model_name, old_index_name, new_index_name): + model_state = self.models[app_label, model_name] + objs = model_state.options["indexes"] + + new_indexes = [] + for obj in objs: + if obj.name == old_index_name: + obj = obj.clone() + obj.name = new_index_name + new_indexes.append(obj) + + model_state.options["indexes"] = new_indexes + self.reload_model(app_label, model_name, delay=True) + def add_constraint(self, app_label, model_name, constraint): self._append_option(app_label, model_name, "constraints", constraint) diff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt index 0f50c1eb8e..f8bfc4fb6b 100644 --- a/docs/ref/migration-operations.txt +++ b/docs/ref/migration-operations.txt @@ -222,6 +222,22 @@ Creates an index in the database table for the model with ``model_name``. Removes the index named ``name`` from the model with ``model_name``. +``RenameIndex`` +--------------- + +.. versionadded:: 4.1 + +.. class:: RenameIndex(model_name, new_name, old_name=None, old_fields=None) + +Renames an index in the database table for the model with ``model_name``. +Exactly one of ``old_name`` and ``old_fields`` can be provided. ``old_fields`` +is an iterable of the strings, often corresponding to fields of +:attr:`~django.db.models.Options.index_together`. + +On databases that don't support an index renaming statement (SQLite and MariaDB +< 10.5.2), the operation will drop and recreate the index, which can be +expensive. + ``AddConstraint`` ----------------- diff --git a/docs/ref/schema-editor.txt b/docs/ref/schema-editor.txt index 719e69826f..99d93f5ab4 100644 --- a/docs/ref/schema-editor.txt +++ b/docs/ref/schema-editor.txt @@ -79,6 +79,15 @@ Adds ``index`` to ``model``’s table. Removes ``index`` from ``model``’s table. +``rename_index()`` +------------------ + +.. versionadded:: 4.1 + +.. method:: BaseDatabaseSchemaEditor.rename_index(model, old_index, new_index) + +Renames ``old_index`` from ``model``’s table to ``new_index``. + ``add_constraint()`` -------------------- diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index 677e8c0345..38dcdb94e0 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -342,7 +342,10 @@ Management Commands Migrations ~~~~~~~~~~ -* ... +* The new :class:`~django.db.migrations.operations.RenameIndex` operation + allows renaming indexes defined in the + :attr:`Meta.indexes ` or + :attr:`~django.db.models.Options.index_together` options. Models ~~~~~~ diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 610a8b53ce..cfd28b1b39 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -2900,6 +2900,120 @@ class OperationTests(OperationTestBase): self.unapply_operations("test_rmin", project_state, operations=operations) self.assertIndexExists("test_rmin_pony", ["pink", "weight"]) + def test_rename_index(self): + app_label = "test_rnin" + project_state = self.set_up_test_model(app_label, index=True) + table_name = app_label + "_pony" + self.assertIndexNameExists(table_name, "pony_pink_idx") + self.assertIndexNameNotExists(table_name, "new_pony_test_idx") + operation = migrations.RenameIndex( + "Pony", new_name="new_pony_test_idx", old_name="pony_pink_idx" + ) + self.assertEqual( + operation.describe(), + "Rename index pony_pink_idx on Pony to new_pony_test_idx", + ) + self.assertEqual( + operation.migration_name_fragment, + "rename_pony_pink_idx_new_pony_test_idx", + ) + + new_state = project_state.clone() + operation.state_forwards(app_label, new_state) + # Rename index. + expected_queries = 1 if connection.features.can_rename_index else 2 + with connection.schema_editor() as editor, self.assertNumQueries( + expected_queries + ): + operation.database_forwards(app_label, editor, project_state, new_state) + self.assertIndexNameNotExists(table_name, "pony_pink_idx") + self.assertIndexNameExists(table_name, "new_pony_test_idx") + # Reversal. + with connection.schema_editor() as editor, self.assertNumQueries( + expected_queries + ): + operation.database_backwards(app_label, editor, new_state, project_state) + self.assertIndexNameExists(table_name, "pony_pink_idx") + self.assertIndexNameNotExists(table_name, "new_pony_test_idx") + # Deconstruction. + definition = operation.deconstruct() + self.assertEqual(definition[0], "RenameIndex") + self.assertEqual(definition[1], []) + self.assertEqual( + definition[2], + { + "model_name": "Pony", + "old_name": "pony_pink_idx", + "new_name": "new_pony_test_idx", + }, + ) + + def test_rename_index_arguments(self): + msg = "RenameIndex.old_name and old_fields are mutually exclusive." + with self.assertRaisesMessage(ValueError, msg): + migrations.RenameIndex( + "Pony", + new_name="new_idx_name", + old_name="old_idx_name", + old_fields=("weight", "pink"), + ) + msg = "RenameIndex requires one of old_name and old_fields arguments to be set." + with self.assertRaisesMessage(ValueError, msg): + migrations.RenameIndex("Pony", new_name="new_idx_name") + + def test_rename_index_unnamed_index(self): + app_label = "test_rninui" + project_state = self.set_up_test_model(app_label, index_together=True) + table_name = app_label + "_pony" + self.assertIndexNameNotExists(table_name, "new_pony_test_idx") + operation = migrations.RenameIndex( + "Pony", new_name="new_pony_test_idx", old_fields=("weight", "pink") + ) + self.assertEqual( + operation.describe(), + "Rename unnamed index for ('weight', 'pink') on Pony to new_pony_test_idx", + ) + self.assertEqual( + operation.migration_name_fragment, + "rename_pony_weight_pink_new_pony_test_idx", + ) + + new_state = project_state.clone() + operation.state_forwards(app_label, new_state) + # Rename index. + with connection.schema_editor() as editor: + operation.database_forwards(app_label, editor, project_state, new_state) + self.assertIndexNameExists(table_name, "new_pony_test_idx") + # Reverse is a no-op. + with connection.schema_editor() as editor, self.assertNumQueries(0): + operation.database_backwards(app_label, editor, new_state, project_state) + self.assertIndexNameExists(table_name, "new_pony_test_idx") + # Deconstruction. + definition = operation.deconstruct() + self.assertEqual(definition[0], "RenameIndex") + self.assertEqual(definition[1], []) + self.assertEqual( + definition[2], + { + "model_name": "Pony", + "new_name": "new_pony_test_idx", + "old_fields": ("weight", "pink"), + }, + ) + + def test_rename_index_unknown_unnamed_index(self): + app_label = "test_rninuui" + project_state = self.set_up_test_model(app_label) + operation = migrations.RenameIndex( + "Pony", new_name="new_pony_test_idx", old_fields=("weight", "pink") + ) + new_state = project_state.clone() + operation.state_forwards(app_label, new_state) + msg = "Found wrong number (0) of indexes for test_rninuui_pony(weight, pink)." + with connection.schema_editor() as editor: + with self.assertRaisesMessage(ValueError, msg): + operation.database_forwards(app_label, editor, project_state, new_state) + def test_add_index_state_forwards(self): project_state = self.set_up_test_model("test_adinsf") index = models.Index(fields=["pink"], name="test_adinsf_pony_pink_idx") @@ -2923,6 +3037,35 @@ class OperationTests(OperationTestBase): new_model = new_state.apps.get_model("test_rminsf", "Pony") self.assertIsNot(old_model, new_model) + def test_rename_index_state_forwards(self): + app_label = "test_rnidsf" + project_state = self.set_up_test_model(app_label, index=True) + old_model = project_state.apps.get_model(app_label, "Pony") + new_state = project_state.clone() + + operation = migrations.RenameIndex( + "Pony", new_name="new_pony_pink_idx", old_name="pony_pink_idx" + ) + operation.state_forwards(app_label, new_state) + new_model = new_state.apps.get_model(app_label, "Pony") + self.assertIsNot(old_model, new_model) + self.assertEqual(new_model._meta.indexes[0].name, "new_pony_pink_idx") + + def test_rename_index_state_forwards_unnamed_index(self): + app_label = "test_rnidsfui" + project_state = self.set_up_test_model(app_label, index_together=True) + old_model = project_state.apps.get_model(app_label, "Pony") + new_state = project_state.clone() + + operation = migrations.RenameIndex( + "Pony", new_name="new_pony_pink_idx", old_fields=("weight", "pink") + ) + operation.state_forwards(app_label, new_state) + new_model = new_state.apps.get_model(app_label, "Pony") + self.assertIsNot(old_model, new_model) + self.assertEqual(new_model._meta.index_together, tuple()) + self.assertEqual(new_model._meta.indexes[0].name, "new_pony_pink_idx") + @skipUnlessDBFeature("supports_expression_indexes") def test_add_func_index(self): app_label = "test_addfuncin" diff --git a/tests/migrations/test_optimizer.py b/tests/migrations/test_optimizer.py index 6bde378cd9..6485009eb4 100644 --- a/tests/migrations/test_optimizer.py +++ b/tests/migrations/test_optimizer.py @@ -1114,3 +1114,41 @@ class OptimizerTests(SimpleTestCase): ), ], ) + + def test_rename_index(self): + self.assertOptimizesTo( + [ + migrations.RenameIndex( + "Pony", new_name="mid_name", old_fields=("weight", "pink") + ), + migrations.RenameIndex( + "Pony", new_name="new_name", old_name="mid_name" + ), + ], + [ + migrations.RenameIndex( + "Pony", new_name="new_name", old_fields=("weight", "pink") + ), + ], + ) + self.assertOptimizesTo( + [ + migrations.RenameIndex( + "Pony", new_name="mid_name", old_name="old_name" + ), + migrations.RenameIndex( + "Pony", new_name="new_name", old_name="mid_name" + ), + ], + [migrations.RenameIndex("Pony", new_name="new_name", old_name="old_name")], + ) + self.assertDoesNotOptimize( + [ + migrations.RenameIndex( + "Pony", new_name="mid_name", old_name="old_name" + ), + migrations.RenameIndex( + "Pony", new_name="new_name", old_fields=("weight", "pink") + ), + ] + )