From 3dac3271d286f2790780e89d31ddbb7197f8defa Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Thu, 1 Aug 2024 09:21:07 +0200 Subject: [PATCH] Reverted "Fixed #28646 -- Prevented duplicate index when unique is set to True on PostgreSQL." This reverts commit 9cf9c796be8dd53bc3b11355ff39d65c81d7be6d due to a crash on Oracle as it didn't allow multiple indexes on the same field. --- django/db/backends/base/schema.py | 4 +- django/db/backends/postgresql/schema.py | 38 +---- tests/migrations/test_base.py | 2 +- tests/migrations/test_operations.py | 47 ------ tests/schema/models.py | 14 -- tests/schema/tests.py | 211 ------------------------ 6 files changed, 11 insertions(+), 305 deletions(-) diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index f6b675339a..3e38c56d50 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -964,7 +964,9 @@ class BaseDatabaseSchemaEditor: fks_dropped.add((old_field.column,)) self.execute(self._delete_fk_sql(model, fk_name)) # Has unique been removed? - if old_field.unique and not old_field.primary_key and not new_field.unique: + if old_field.unique and ( + not new_field.unique or self._field_became_primary_key(old_field, new_field) + ): # Find the unique constraint for this field meta_constraint_names = { constraint.name for constraint in model._meta.constraints diff --git a/django/db/backends/postgresql/schema.py b/django/db/backends/postgresql/schema.py index c0a785c7e1..0c8548a5d6 100644 --- a/django/db/backends/postgresql/schema.py +++ b/django/db/backends/postgresql/schema.py @@ -140,13 +140,6 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): return sequence["name"] return None - def _is_changing_type_of_indexed_text_column(self, old_field, old_type, new_type): - return (old_field.db_index or old_field.unique) and ( - (old_type.startswith("varchar") and not new_type.startswith("varchar")) - or (old_type.startswith("text") and not new_type.startswith("text")) - or (old_type.startswith("citext") and not new_type.startswith("citext")) - ) - def _alter_column_type_sql( self, model, old_field, new_field, new_type, old_collation, new_collation ): @@ -154,7 +147,11 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): # different type. old_db_params = old_field.db_parameters(connection=self.connection) old_type = old_db_params["type"] - if self._is_changing_type_of_indexed_text_column(old_field, old_type, new_type): + if (old_field.db_index or old_field.unique) and ( + (old_type.startswith("varchar") and not new_type.startswith("varchar")) + or (old_type.startswith("text") and not new_type.startswith("text")) + or (old_type.startswith("citext") and not new_type.startswith("citext")) + ): index_name = self._create_index_name( model._meta.db_table, [old_field.column], suffix="_like" ) @@ -258,25 +255,6 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): model, old_field, new_field, new_type, old_collation, new_collation ) - def _new_index_should_be_added(self, old_field, new_field): - return not (old_field.db_index or old_field.unique) and ( - new_field.db_index or new_field.unique - ) - - def _deleted_index_should_be_recreated( - self, old_field, new_field, old_type, new_type - ): - if ( - not old_field.unique - and ( - not new_field.db_index - or (new_field.unique and not new_field.primary_key) - ) - ) or ( - self._is_changing_type_of_indexed_text_column(old_field, old_type, new_type) - ): - return True - def _alter_field( self, model, @@ -299,10 +277,8 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): strict, ) # Added an index? Create any PostgreSQL-specific indexes. - if self._new_index_should_be_added( - old_field, new_field - ) or self._deleted_index_should_be_recreated( - old_field, new_field, old_type, new_type + if (not (old_field.db_index or old_field.unique) and new_field.db_index) or ( + not old_field.unique and new_field.unique ): like_index_statement = self._create_like_index_sql(model, new_field) if like_index_statement is not None: diff --git a/tests/migrations/test_base.py b/tests/migrations/test_base.py index 1f565eb03d..0ff1dda1d9 100644 --- a/tests/migrations/test_base.py +++ b/tests/migrations/test_base.py @@ -102,7 +102,7 @@ class MigrationTestBase(TransactionTestCase): .values() if ( c["columns"] == list(columns) - and (index_type is None or c.get("type") == index_type) + and (index_type is None or c["type"] == index_type) and not c["unique"] ) ), diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 42629e3494..3ac813b899 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1,5 +1,4 @@ import math -import unittest from decimal import Decimal from django.core.exceptions import FieldDoesNotExist @@ -6214,52 +6213,6 @@ class OperationTests(OperationTestBase): self.assertEqual(pony_new.static, 2) -class PrimaryKeyOperations(OperationTestBase): - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_slugfields_change_primary_key_operations(self): - operation1 = migrations.CreateModel( - "SimpleModel", - [ - ("field1", models.SlugField(max_length=20, primary_key=True)), - ("field2", models.SlugField(max_length=20)), - ], - ) - operation2 = migrations.AlterField( - "SimpleModel", - "field1", - models.SlugField(max_length=20, primary_key=False), - ) - operation3 = migrations.AlterField( - "SimpleModel", - "field2", - models.SlugField(max_length=20, primary_key=True), - ) - project_state = ProjectState() - new_state = project_state.clone() - operation1.state_forwards("migrtest", new_state) - - self.assertTableNotExists("migrtest_simplemodel") - with connection.schema_editor() as editor: - operation1.state_forwards("migrtest", new_state) - operation1.database_forwards("migrtest", editor, project_state, new_state) - project_state, new_state = new_state, new_state.clone() - operation2.state_forwards("migrtest", new_state) - operation2.database_forwards("migrtest", editor, project_state, new_state) - project_state, new_state = new_state, new_state.clone() - operation3.state_forwards("migrtest", new_state) - operation3.database_forwards("migrtest", editor, project_state, new_state) - self.assertTableExists("migrtest_simplemodel") - self.assertColumnExists("migrtest_simplemodel", "field1") - self.assertColumnExists("migrtest_simplemodel", "field2") - with connection.cursor() as cursor: - primary_keys = connection.introspection.get_primary_key_columns( - cursor, "migrtest_simplemodel" - ) - self.assertEqual(["field2"], primary_keys) - self.assertIndexExists("migrtest_simplemodel", ["field1"], index_type="idx") - self.assertIndexExists("migrtest_simplemodel", ["field2"], index_type="idx") - - class SwappableOperationTests(OperationTestBase): """ Key operations ignore swappable models diff --git a/tests/schema/models.py b/tests/schema/models.py index 2fc9095b61..75e32a0eab 100644 --- a/tests/schema/models.py +++ b/tests/schema/models.py @@ -147,20 +147,6 @@ class IntegerPK(models.Model): db_table = "INTEGERPK" # uppercase to ensure proper quoting -class CharFieldPK(models.Model): - field1 = models.CharField(max_length=10, primary_key=True) - - class Meta: - apps = new_apps - - -class CharFieldPKUnique(models.Model): - field1 = models.CharField(max_length=10, primary_key=True, unique=True) - - class Meta: - apps = new_apps - - class Note(models.Model): info = models.TextField() address = models.TextField(null=True) diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 88e346e923..3a2947cf43 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -86,8 +86,6 @@ from .models import ( BookWithO2O, BookWithoutAuthor, BookWithSlug, - CharFieldPK, - CharFieldPKUnique, IntegerPK, Node, Note, @@ -123,8 +121,6 @@ class SchemaTests(TransactionTestCase): BookWithLongName, BookWithO2O, BookWithSlug, - CharFieldPK, - CharFieldPKUnique, IntegerPK, Node, Note, @@ -5214,213 +5210,6 @@ class SchemaTests(TransactionTestCase): ["schema_tag_slug_2c418ba3_like", "schema_tag_slug_key"], ) - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_charfield_primary_key_to_db_index(self): - # Create the table and verify initial indexes. - with connection.schema_editor() as editor: - editor.create_model(CharFieldPK) - self.assertEqual( - self.get_constraints_for_column(CharFieldPK, "field1"), - ["schema_charfieldpk_field1_0eb93b91_like", "schema_charfieldpk_pkey"], - ) - # Alter to remove primary_key and set db_index=True. - old_field1 = CharFieldPK._meta.get_field("field1") - new_field1 = CharField(db_index=True, primary_key=False) - new_field1.set_attributes_from_name("field1") - with connection.schema_editor() as editor: - editor.alter_field(CharFieldPK, old_field1, new_field1, strict=True) - self.assertEqual( - self.get_constraints_for_column(CharFieldPK, "field1"), - [ - "schema_charfieldpk_field1_0eb93b91", - "schema_charfieldpk_field1_0eb93b91_like", - ], - ) - - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_charfield_primary_key_to_unique(self): - # Create the table and verify initial indexes. - with connection.schema_editor() as editor: - editor.create_model(CharFieldPK) - self.assertEqual( - self.get_constraints_for_column(CharFieldPK, "field1"), - ["schema_charfieldpk_field1_0eb93b91_like", "schema_charfieldpk_pkey"], - ) - # Alter to remove primary_key and set unique=True. - old_field1 = CharFieldPK._meta.get_field("field1") - new_field1 = CharField(unique=True, primary_key=False) - new_field1.set_attributes_from_name("field1") - new_field1.model = CharFieldPK - with connection.schema_editor() as editor: - editor.alter_field(CharFieldPK, old_field1, new_field1, strict=True) - self.assertEqual( - self.get_constraints_for_column(CharFieldPK, "field1"), - [ - "schema_charfieldpk_field1_0eb93b91_like", - "schema_charfieldpk_field1_0eb93b91_uniq", - ], - ) - - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_charfield_primary_key_to_db_index_and_unique(self): - # Create the table and verify initial indexes. - with connection.schema_editor() as editor: - editor.create_model(CharFieldPK) - self.assertEqual( - self.get_constraints_for_column(CharFieldPK, "field1"), - ["schema_charfieldpk_field1_0eb93b91_like", "schema_charfieldpk_pkey"], - ) - # Alter to remove primary_key and set db_index=True and unique=True. - old_field1 = CharFieldPK._meta.get_field("field1") - new_field1 = CharField(unique=True, db_index=True, primary_key=False) - new_field1.set_attributes_from_name("field1") - new_field1.model = CharFieldPK - with connection.schema_editor() as editor: - editor.alter_field(CharFieldPK, old_field1, new_field1, strict=True) - self.assertEqual( - self.get_constraints_for_column(CharFieldPK, "field1"), - [ - "schema_charfieldpk_field1_0eb93b91_like", - "schema_charfieldpk_field1_0eb93b91_uniq", - ], - ) - - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_charfield_primary_key_unique_to_just_unique(self): - # Create the table and verify initial indexes. - with connection.schema_editor() as editor: - editor.create_model(CharFieldPKUnique) - self.assertEqual( - self.get_constraints_for_column(CharFieldPKUnique, "field1"), - [ - "schema_charfieldpkunique_field1_ffc9a22c_like", - "schema_charfieldpkunique_pkey", - ], - ) - # Alter to remove primary_key (but still unique). - old_field1 = CharFieldPKUnique._meta.get_field("field1") - new_field1 = CharField(unique=True, primary_key=False) - new_field1.set_attributes_from_name("field1") - new_field1.model = CharFieldPKUnique - with connection.schema_editor() as editor: - editor.alter_field(CharFieldPKUnique, old_field1, new_field1, strict=True) - self.assertEqual( - self.get_constraints_for_column(CharFieldPKUnique, "field1"), - [ - "schema_charfieldpkunique_field1_ffc9a22c_like", - "schema_charfieldpkunique_field1_ffc9a22c_uniq", - ], - ) - - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_charfield_db_index_to_textfield_db_index(self): - with connection.schema_editor() as editor: - editor.create_model(AuthorCharFieldWithIndex) - self.assertEqual( - self.get_constraints_for_column(AuthorCharFieldWithIndex, "char_field"), - [ - "schema_authorcharfieldwithindex_char_field_06a11776", - "schema_authorcharfieldwithindex_char_field_06a11776_like", - ], - ) - old_field = AuthorCharFieldWithIndex._meta.get_field("char_field") - new_field = TextField(db_index=True) - new_field.set_attributes_from_name("char_field") - new_field.model = AuthorCharFieldWithIndex - with connection.schema_editor() as editor: - editor.alter_field( - AuthorCharFieldWithIndex, old_field, new_field, strict=True - ) - self.assertEqual( - self.get_constraints_for_column(AuthorCharFieldWithIndex, "char_field"), - [ - "schema_authorcharfieldwithindex_char_field_06a11776", - "schema_authorcharfieldwithindex_char_field_06a11776_like", - ], - ) - - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_charfield_change_max_length_no_index_created(self): - with connection.schema_editor() as editor: - editor.create_model(Author) - self.assertEqual( - self.get_constraints_for_column(Author, "name"), - [], - ) - old_name = Author._meta.get_field("name") - new_name = CharField(max_length=20) - new_name.set_attributes_from_name("name") - with connection.schema_editor() as editor: - editor.alter_field(Author, old_name, new_name, strict=True) - self.assertEqual( - self.get_constraints_for_column(Author, "name"), - [], - ) - - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_remove_db_index(self): - with connection.schema_editor() as editor: - editor.create_model(AuthorCharFieldWithIndex) - self.assertEqual( - self.get_constraints_for_column(AuthorCharFieldWithIndex, "char_field"), - [ - "schema_authorcharfieldwithindex_char_field_06a11776", - "schema_authorcharfieldwithindex_char_field_06a11776_like", - ], - ) - old_char_field = AuthorCharFieldWithIndex._meta.get_field("char_field") - new_char_field = CharField(max_length=31, db_index=False) - new_char_field.set_attributes_from_name("char_field") - with connection.schema_editor() as editor: - editor.alter_field( - AuthorCharFieldWithIndex, old_char_field, new_char_field, strict=True - ) - self.assertEqual( - self.get_constraints_for_column(AuthorCharFieldWithIndex, "char_field"), - [], - ) - - @isolate_apps("schema") - @unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL specific") - def test_slugfields_change_primary_key(self): - class SimpleModel(Model): - field1 = SlugField(max_length=20, primary_key=True) - field2 = SlugField(max_length=20) - - class Meta: - app_label = "schema" - - with connection.schema_editor() as editor: - editor.create_model(SimpleModel) - self.assertEqual( - self.get_constraints_for_column(SimpleModel, "field1"), - [ - "schema_simplemodel_field1_f07a3d6a_like", - "schema_simplemodel_pkey", - ], - ) - # Remove primary_key from field1. - old_field1 = SimpleModel._meta.get_field("field1") - new_field1 = CharField(max_length=20, primary_key=False) - new_field1.set_attributes_from_name("field1") - with connection.schema_editor() as editor: - editor.alter_field(SimpleModel, old_field1, new_field1, strict=True) - self.assertEqual(self.get_constraints_for_column(SimpleModel, "field1"), []) - # Add primary_key to field2. - old_field2 = SimpleModel._meta.get_field("field2") - new_field2 = CharField(max_length=20, primary_key=True) - new_field2.set_attributes_from_name("field2") - new_field2.model = SimpleModel - with connection.schema_editor() as editor: - editor.alter_field(SimpleModel, old_field2, new_field2, strict=True) - self.assertEqual( - self.get_constraints_for_column(SimpleModel, "field2"), - [ - "schema_simplemodel_field2_08772539_like", - "schema_simplemodel_field2_08772539_pk", - ], - ) - def test_alter_field_add_index_to_integerfield(self): # Create the table and verify no initial indexes. with connection.schema_editor() as editor: