Fixed #34219 -- Preserved Char/TextField.db_collation when altering column type.

This moves setting a database collation to the column type alteration
as both must be set at the same time.

This should also avoid another layer of the column type alteration when
adding database comments support (#18468).

django/contrib/gis/db/backends/postgis/schema.py

@@ -50,12 +50,16 @@ class PostGISSchemaEditor(DatabaseSchemaEditor):
             expressions=expressions,
         )
 
-    def _alter_column_type_sql(self, table, old_field, new_field, new_type):
+    def _alter_column_type_sql(
+        self, table, old_field, new_field, new_type, old_collation, new_collation
+    ):
         """
         Special case when dimension changed.
         """
         if not hasattr(old_field, "dim") or not hasattr(new_field, "dim"):
-            return super()._alter_column_type_sql(table, old_field, new_field, new_type)
+            return super()._alter_column_type_sql(
+                table, old_field, new_field, new_type, old_collation, new_collation
+            )
 
         if old_field.dim == 2 and new_field.dim == 3:
             sql_alter = self.sql_alter_column_to_3d
@@ -69,6 +73,7 @@ class PostGISSchemaEditor(DatabaseSchemaEditor):
                 % {
                     "column": self.quote_name(new_field.column),
                     "type": new_type,
+                    "collation": "",
                 },
                 [],
             ),

django/db/backends/base/schema.py

@@ -87,13 +87,12 @@ class BaseDatabaseSchemaEditor:
 
     sql_create_column = "ALTER TABLE %(table)s ADD COLUMN %(column)s %(definition)s"
     sql_alter_column = "ALTER TABLE %(table)s %(changes)s"
-    sql_alter_column_type = "ALTER COLUMN %(column)s TYPE %(type)s"
+    sql_alter_column_type = "ALTER COLUMN %(column)s TYPE %(type)s%(collation)s"
     sql_alter_column_null = "ALTER COLUMN %(column)s DROP NOT NULL"
     sql_alter_column_not_null = "ALTER COLUMN %(column)s SET NOT NULL"
     sql_alter_column_default = "ALTER COLUMN %(column)s SET DEFAULT %(default)s"
     sql_alter_column_no_default = "ALTER COLUMN %(column)s DROP DEFAULT"
     sql_alter_column_no_default_null = sql_alter_column_no_default
-    sql_alter_column_collate = "ALTER COLUMN %(column)s TYPE %(type)s%(collation)s"
     sql_delete_column = "ALTER TABLE %(table)s DROP COLUMN %(column)s CASCADE"
     sql_rename_column = (
         "ALTER TABLE %(table)s RENAME COLUMN %(old_column)s TO %(new_column)s"
@@ -950,17 +949,14 @@ class BaseDatabaseSchemaEditor:
         # Type suffix change? (e.g. auto increment).
         old_type_suffix = old_field.db_type_suffix(connection=self.connection)
         new_type_suffix = new_field.db_type_suffix(connection=self.connection)
-        # Collation change?
+        # Type or collation change?
-        if old_collation != new_collation:
+        if (
-            # Collation change handles also a type change.
+            old_type != new_type
-            fragment = self._alter_column_collation_sql(
+            or old_type_suffix != new_type_suffix
-                model, new_field, new_type, new_collation, old_field
+            or old_collation != new_collation
-            )
+        ):
-            actions.append(fragment)
-        # Type change?
-        elif (old_type, old_type_suffix) != (new_type, new_type_suffix):
             fragment, other_actions = self._alter_column_type_sql(
-                model, old_field, new_field, new_type
+                model, old_field, new_field, new_type, old_collation, new_collation
             )
             actions.append(fragment)
             post_actions.extend(other_actions)
@@ -1076,20 +1072,14 @@ class BaseDatabaseSchemaEditor:
             rel_collation = rel_db_params.get("collation")
             old_rel_db_params = old_rel.field.db_parameters(connection=self.connection)
             old_rel_collation = old_rel_db_params.get("collation")
-            if old_rel_collation != rel_collation:
+            fragment, other_actions = self._alter_column_type_sql(
-                # Collation change handles also a type change.
+                new_rel.related_model,
-                fragment = self._alter_column_collation_sql(
+                old_rel.field,
-                    new_rel.related_model,
+                new_rel.field,
-                    new_rel.field,
+                rel_type,
-                    rel_type,
+                old_rel_collation,
-                    rel_collation,
+                rel_collation,
-                    old_rel.field,
+            )
-                )
-                other_actions = []
-            else:
-                fragment, other_actions = self._alter_column_type_sql(
-                    new_rel.related_model, old_rel.field, new_rel.field, rel_type
-                )
             self.execute(
                 self.sql_alter_column
                 % {
@@ -1209,7 +1199,9 @@ class BaseDatabaseSchemaEditor:
             params,
         )
 
-    def _alter_column_type_sql(self, model, old_field, new_field, new_type):
+    def _alter_column_type_sql(
+        self, model, old_field, new_field, new_type, old_collation, new_collation
+    ):
         """
         Hook to specialize column type alteration for different backends,
         for cases when a creation type is different to an alteration type
@@ -1219,33 +1211,25 @@ class BaseDatabaseSchemaEditor:
         an ALTER TABLE statement and a list of extra (sql, params) tuples to
         run once the field is altered.
         """
+        if collate_sql := self._collate_sql(
+            new_collation, old_collation, model._meta.db_table
+        ):
+            collate_sql = f" {collate_sql}"
+        else:
+            collate_sql = ""
         return (
             (
                 self.sql_alter_column_type
                 % {
                     "column": self.quote_name(new_field.column),
                     "type": new_type,
+                    "collation": collate_sql,
                 },
                 [],
             ),
             [],
         )
 
-    def _alter_column_collation_sql(
-        self, model, new_field, new_type, new_collation, old_field
-    ):
-        return (
-            self.sql_alter_column_collate
-            % {
-                "column": self.quote_name(new_field.column),
-                "type": new_type,
-                "collation": " " + self._collate_sql(new_collation)
-                if new_collation
-                else "",
-            },
-            [],
-        )
-
     def _alter_many_to_many(self, model, old_field, new_field, strict):
         """Alter M2Ms to repoint their to= endpoints."""
         # Rename the through table
@@ -1745,8 +1729,8 @@ class BaseDatabaseSchemaEditor:
     def _delete_primary_key_sql(self, model, name):
         return self._delete_constraint_sql(self.sql_delete_pk, model, name)
 
-    def _collate_sql(self, collation):
+    def _collate_sql(self, collation, old_collation=None, table_name=None):
-        return "COLLATE " + self.quote_name(collation)
+        return "COLLATE " + self.quote_name(collation) if collation else ""
 
     def remove_procedure(self, procedure_name, param_types=()):
         sql = self.sql_delete_procedure % {

django/db/backends/mysql/schema.py

@@ -9,8 +9,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
 
     sql_alter_column_null = "MODIFY %(column)s %(type)s NULL"
     sql_alter_column_not_null = "MODIFY %(column)s %(type)s NOT NULL"
-    sql_alter_column_type = "MODIFY %(column)s %(type)s"
+    sql_alter_column_type = "MODIFY %(column)s %(type)s%(collation)s"
-    sql_alter_column_collate = "MODIFY %(column)s %(type)s%(collation)s"
     sql_alter_column_no_default_null = "ALTER COLUMN %(column)s SET DEFAULT NULL"
 
     # No 'CASCADE' which works as a no-op in MySQL but is undocumented
@@ -218,9 +217,13 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             new_type += " NOT NULL"
         return new_type
 
-    def _alter_column_type_sql(self, model, old_field, new_field, new_type):
+    def _alter_column_type_sql(
+        self, model, old_field, new_field, new_type, old_collation, new_collation
+    ):
         new_type = self._set_field_new_type_null_status(old_field, new_type)
-        return super()._alter_column_type_sql(model, old_field, new_field, new_type)
+        return super()._alter_column_type_sql(
+            model, old_field, new_field, new_type, old_collation, new_collation
+        )
 
     def _rename_field_sql(self, table, old_field, new_field, new_type):
         new_type = self._set_field_new_type_null_status(old_field, new_type)

django/db/backends/oracle/schema.py

@@ -13,13 +13,12 @@ from django.utils.duration import duration_iso_string
 class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
 
     sql_create_column = "ALTER TABLE %(table)s ADD %(column)s %(definition)s"
-    sql_alter_column_type = "MODIFY %(column)s %(type)s"
+    sql_alter_column_type = "MODIFY %(column)s %(type)s%(collation)s"
     sql_alter_column_null = "MODIFY %(column)s NULL"
     sql_alter_column_not_null = "MODIFY %(column)s NOT NULL"
     sql_alter_column_default = "MODIFY %(column)s DEFAULT %(default)s"
     sql_alter_column_no_default = "MODIFY %(column)s DEFAULT NULL"
     sql_alter_column_no_default_null = sql_alter_column_no_default
-    sql_alter_column_collate = "MODIFY %(column)s %(type)s%(collation)s"
 
     sql_delete_column = "ALTER TABLE %(table)s DROP COLUMN %(column)s"
     sql_create_column_inline_fk = (
@@ -169,7 +168,9 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
                         self._create_fk_sql(rel.related_model, rel.field, "_fk")
                     )
 
-    def _alter_column_type_sql(self, model, old_field, new_field, new_type):
+    def _alter_column_type_sql(
+        self, model, old_field, new_field, new_type, old_collation, new_collation
+    ):
         auto_field_types = {"AutoField", "BigAutoField", "SmallAutoField"}
         # Drop the identity if migrating away from AutoField.
         if (
@@ -178,7 +179,9 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             and self._is_identity_column(model._meta.db_table, new_field.column)
         ):
             self._drop_identity(model._meta.db_table, new_field.column)
-        return super()._alter_column_type_sql(model, old_field, new_field, new_type)
+        return super()._alter_column_type_sql(
+            model, old_field, new_field, new_type, old_collation, new_collation
+        )
 
     def normalize_name(self, name):
         """
@@ -242,11 +245,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             )
             return cursor.fetchone()[0]
 
-    def _alter_column_collation_sql(
+    def _collate_sql(self, collation, old_collation=None, table_name=None):
-        self, model, new_field, new_type, new_collation, old_field
+        if collation is None and old_collation is not None:
-    ):
+            collation = self._get_default_collation(table_name)
-        if new_collation is None:
+        return super()._collate_sql(collation, old_collation, table_name)
-            new_collation = self._get_default_collation(model._meta.db_table)
-        return super()._alter_column_collation_sql(
-            model, new_field, new_type, new_collation, old_field
-        )

django/db/backends/postgresql/schema.py

@@ -140,7 +140,9 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
                     return sequence["name"]
         return None
 
-    def _alter_column_type_sql(self, model, old_field, new_field, new_type):
+    def _alter_column_type_sql(
+        self, model, old_field, new_field, new_type, old_collation, new_collation
+    ):
         # Drop indexes on varchar/text/citext columns that are changing to a
         # different type.
         old_db_params = old_field.db_parameters(connection=self.connection)
@@ -155,7 +157,9 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             )
             self.execute(self._delete_index_sql(model, index_name))
 
-        self.sql_alter_column_type = "ALTER COLUMN %(column)s TYPE %(type)s"
+        self.sql_alter_column_type = (
+            "ALTER COLUMN %(column)s TYPE %(type)s%(collation)s"
+        )
         # Cast when data type changed.
         if using_sql := self._using_sql(new_field, old_field):
             self.sql_alter_column_type += using_sql
@@ -178,6 +182,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
                     % {
                         "column": self.quote_name(column),
                         "type": new_type,
+                        "collation": "",
                     },
                     [],
                 ),
@@ -204,7 +209,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             )
             column = strip_quotes(new_field.column)
             fragment, _ = super()._alter_column_type_sql(
-                model, old_field, new_field, new_type
+                model, old_field, new_field, new_type, old_collation, new_collation
             )
             # Drop the sequence if exists (Django 4.1+ identity columns don't
             # have it).
@@ -222,7 +227,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             return fragment, other_actions
         elif new_is_auto and old_is_auto and old_internal_type != new_internal_type:
             fragment, _ = super()._alter_column_type_sql(
-                model, old_field, new_field, new_type
+                model, old_field, new_field, new_type, old_collation, new_collation
             )
             column = strip_quotes(new_field.column)
             db_types = {
@@ -246,7 +251,9 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
                 ]
             return fragment, other_actions
         else:
-            return super()._alter_column_type_sql(model, old_field, new_field, new_type)
+            return super()._alter_column_type_sql(
+                model, old_field, new_field, new_type, old_collation, new_collation
+            )
 
     def _alter_column_collation_sql(
         self, model, new_field, new_type, new_collation, old_field

tests/schema/tests.py

@@ -4950,6 +4950,38 @@ class SchemaTests(TransactionTestCase):
             editor.alter_field(Author, new_field, old_field, strict=True)
         self.assertIsNone(self.get_column_collation(Author._meta.db_table, "name"))
 
+    @skipUnlessDBFeature("supports_collation_on_charfield")
+    def test_alter_field_type_preserve_db_collation(self):
+        collation = connection.features.test_collations.get("non_default")
+        if not collation:
+            self.skipTest("Language collations are not supported.")
+
+        with connection.schema_editor() as editor:
+            editor.create_model(Author)
+
+        old_field = Author._meta.get_field("name")
+        new_field = CharField(max_length=255, db_collation=collation)
+        new_field.set_attributes_from_name("name")
+        new_field.model = Author
+        with connection.schema_editor() as editor:
+            editor.alter_field(Author, old_field, new_field, strict=True)
+        self.assertEqual(
+            self.get_column_collation(Author._meta.db_table, "name"),
+            collation,
+        )
+        # Changing a field type should preserve the collation.
+        old_field = new_field
+        new_field = CharField(max_length=511, db_collation=collation)
+        new_field.set_attributes_from_name("name")
+        new_field.model = Author
+        with connection.schema_editor() as editor:
+            editor.alter_field(Author, new_field, old_field, strict=True)
+        # Collation is preserved.
+        self.assertEqual(
+            self.get_column_collation(Author._meta.db_table, "name"),
+            collation,
+        )
+
     @skipUnlessDBFeature("supports_collation_on_charfield")
     def test_alter_primary_key_db_collation(self):
         collation = connection.features.test_collations.get("non_default")