diff options
| -rw-r--r-- | django/db/backends/base/schema.py | 4 | ||||
| -rw-r--r-- | django/db/backends/postgresql/schema.py | 38 | ||||
| -rw-r--r-- | tests/migrations/test_base.py | 2 | ||||
| -rw-r--r-- | tests/migrations/test_operations.py | 47 | ||||
| -rw-r--r-- | tests/schema/models.py | 14 | ||||
| -rw-r--r-- | tests/schema/tests.py | 211 |
6 files changed, 305 insertions, 11 deletions
diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 3e38c56d50..f6b675339a 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -964,9 +964,7 @@ 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 new_field.unique or self._field_became_primary_key(old_field, new_field) - ): + if old_field.unique and not old_field.primary_key and not new_field.unique: # 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 0c8548a5d6..c0a785c7e1 100644 --- a/django/db/backends/postgresql/schema.py +++ b/django/db/backends/postgresql/schema.py @@ -140,6 +140,13 @@ 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 ): @@ -147,11 +154,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): # different type. old_db_params = old_field.db_parameters(connection=self.connection) old_type = old_db_params["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")) - ): + if self._is_changing_type_of_indexed_text_column(old_field, old_type, new_type): index_name = self._create_index_name( model._meta.db_table, [old_field.column], suffix="_like" ) @@ -255,6 +258,25 @@ 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, @@ -277,8 +299,10 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): strict, ) # Added an index? Create any PostgreSQL-specific indexes. - if (not (old_field.db_index or old_field.unique) and new_field.db_index) or ( - not old_field.unique and new_field.unique + 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 ): 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 0ff1dda1d9..1f565eb03d 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["type"] == index_type) + and (index_type is None or c.get("type") == index_type) and not c["unique"] ) ), diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 3ac813b899..42629e3494 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1,4 +1,5 @@ import math +import unittest from decimal import Decimal from django.core.exceptions import FieldDoesNotExist @@ -6213,6 +6214,52 @@ 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 75e32a0eab..2fc9095b61 100644 --- a/tests/schema/models.py +++ b/tests/schema/models.py @@ -147,6 +147,20 @@ 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 3a2947cf43..88e346e923 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -86,6 +86,8 @@ from .models import ( BookWithO2O, BookWithoutAuthor, BookWithSlug, + CharFieldPK, + CharFieldPKUnique, IntegerPK, Node, Note, @@ -121,6 +123,8 @@ class SchemaTests(TransactionTestCase): BookWithLongName, BookWithO2O, BookWithSlug, + CharFieldPK, + CharFieldPKUnique, IntegerPK, Node, Note, @@ -5210,6 +5214,213 @@ 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: |