From 91a4b9a8ec2237434f06866f39c7977e889aeae6 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Thu, 2 May 2024 20:31:51 +0200 Subject: [PATCH] Fixed #35422 -- Fixed migrations crash when altering GeneratedField referencing rename field. Thanks Sarah Boyce for the report and Simon Charette for the implementation idea. --- django/db/backends/base/schema.py | 20 +++++++++---- django/db/backends/mysql/features.py | 12 ++++++++ docs/releases/5.0.5.txt | 3 ++ tests/migrations/test_operations.py | 44 ++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 6 deletions(-) diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 116c98f432c..38136e7213c 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -3,6 +3,7 @@ import operator from datetime import datetime from django.conf import settings +from django.core.exceptions import FieldError from django.db.backends.ddl_references import ( Columns, Expressions, @@ -831,6 +832,7 @@ class BaseDatabaseSchemaEditor: old_type = old_db_params["type"] new_db_params = new_field.db_parameters(connection=self.connection) new_type = new_db_params["type"] + modifying_generated_field = False if (old_type is None and old_field.remote_field is None) or ( new_type is None and new_field.remote_field is None ): @@ -869,13 +871,19 @@ class BaseDatabaseSchemaEditor: "through= on M2M fields)" % (old_field, new_field) ) elif old_field.generated != new_field.generated or ( - new_field.generated - and ( - old_field.db_persist != new_field.db_persist - or old_field.generated_sql(self.connection) - != new_field.generated_sql(self.connection) - ) + new_field.generated and old_field.db_persist != new_field.db_persist ): + modifying_generated_field = True + elif new_field.generated: + try: + old_field_sql = old_field.generated_sql(self.connection) + except FieldError: + # Field used in a generated field was renamed. + modifying_generated_field = True + else: + new_field_sql = new_field.generated_sql(self.connection) + modifying_generated_field = old_field_sql != new_field_sql + if modifying_generated_field: raise ValueError( f"Modifying GeneratedFields is not supported - the field {new_field} " "must be removed and re-added with the new definition." diff --git a/django/db/backends/mysql/features.py b/django/db/backends/mysql/features.py index cafc6702ebb..21088544acf 100644 --- a/django/db/backends/mysql/features.py +++ b/django/db/backends/mysql/features.py @@ -158,6 +158,18 @@ class DatabaseFeatures(BaseDatabaseFeatures): }, } ) + if not self.connection.mysql_is_mariadb: + skips.update( + { + "MySQL doesn't allow renaming columns referenced by generated " + "columns": { + "migrations.test_operations.OperationTests." + "test_invalid_generated_field_changes_on_rename_stored", + "migrations.test_operations.OperationTests." + "test_invalid_generated_field_changes_on_rename_virtual", + }, + } + ) return skips @cached_property diff --git a/docs/releases/5.0.5.txt b/docs/releases/5.0.5.txt index 5421cc9fec8..c12498173d3 100644 --- a/docs/releases/5.0.5.txt +++ b/docs/releases/5.0.5.txt @@ -27,3 +27,6 @@ Bugfixes * Fixed a bug in Django 5.0 that caused a migration crash when a ``GeneratedField`` was added before any of the referenced fields from its ``expression`` definition (:ticket:`35359`). + +* Fixed a bug in Django 5.0 that caused a migration crash when altering a + ``GeneratedField`` referencing a rename field (:ticket:`35422`). diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 5767fbc42f9..d59548f7af5 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -5890,6 +5890,50 @@ class OperationTests(OperationTestBase): def test_invalid_generated_field_changes_virtual(self): self._test_invalid_generated_field_changes(db_persist=False) + def _test_invalid_generated_field_changes_on_rename(self, db_persist): + app_label = "test_igfcor" + operation = migrations.AddField( + "Pony", + "modified_pink", + models.GeneratedField( + expression=F("pink") + F("pink"), + output_field=models.IntegerField(), + db_persist=db_persist, + ), + ) + project_state, new_state = self.make_test_state(app_label, operation) + # Add generated column. + with connection.schema_editor() as editor: + operation.database_forwards(app_label, editor, project_state, new_state) + # Rename field used in the generated field. + operations = [ + migrations.RenameField("Pony", "pink", "renamed_pink"), + migrations.AlterField( + "Pony", + "modified_pink", + models.GeneratedField( + expression=F("renamed_pink"), + output_field=models.IntegerField(), + db_persist=db_persist, + ), + ), + ] + msg = ( + "Modifying GeneratedFields is not supported - the field " + f"{app_label}.Pony.modified_pink must be removed and re-added with the " + "new definition." + ) + with self.assertRaisesMessage(ValueError, msg): + self.apply_operations(app_label, new_state, operations) + + @skipUnlessDBFeature("supports_stored_generated_columns") + def test_invalid_generated_field_changes_on_rename_stored(self): + self._test_invalid_generated_field_changes_on_rename(db_persist=True) + + @skipUnlessDBFeature("supports_virtual_generated_columns") + def test_invalid_generated_field_changes_on_rename_virtual(self): + self._test_invalid_generated_field_changes_on_rename(db_persist=False) + @skipUnlessDBFeature( "supports_stored_generated_columns", "supports_virtual_generated_columns",