From fead2dd52303505f30007df5e3c198b48b3ce9ae Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Tue, 2 Apr 2024 16:33:31 -0400 Subject: [PATCH] [5.0.x] Fixed #35336 -- Addressed crash when adding a GeneratedField with % literals. A longer term solution is likely to have a better separation of parametrized DDL altogether to handle checks, constraints, defaults, and generated fields but such a change would require a significant refactor that isn't suitable for a backport. Thanks Adrian Garcia for the report. Backport of 888b9042b3598bab6557c62de82505eec9ea62ed from main --- django/db/backends/base/schema.py | 23 +++++++++------ django/db/backends/oracle/schema.py | 4 +-- docs/releases/5.0.4.txt | 4 +++ tests/schema/tests.py | 44 ++++++++++++++++++++++++++++- 4 files changed, 62 insertions(+), 13 deletions(-) diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 86bf8b821a..59bf0d16c7 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -317,9 +317,9 @@ class BaseDatabaseSchemaEditor: if default_value is not None: column_default = "DEFAULT " + self._column_default_sql(field) if self.connection.features.requires_literal_defaults: - # Some databases can't take defaults as a parameter (Oracle). - # If this is the case, the individual schema backend should - # implement prepare_default(). + # Some databases can't take defaults as a parameter + # (Oracle, SQLite). If this is the case, the individual + # schema backend should implement prepare_default(). yield column_default % self.prepare_default(default_value) else: yield column_default @@ -333,7 +333,9 @@ class BaseDatabaseSchemaEditor: ): null = True if field.generated: - yield self._column_generated_sql(field) + generated_sql, generated_params = self._column_generated_sql(field) + params.extend(generated_params) + yield generated_sql elif not null: yield "NOT NULL" elif not self.connection.features.implied_column_null: @@ -420,7 +422,7 @@ class BaseDatabaseSchemaEditor: compiler = query.get_compiler(connection=self.connection) default_sql, params = compiler.compile(db_default) if self.connection.features.requires_literal_defaults: - # Some databases doesn't support parameterized defaults (Oracle, + # Some databases don't support parameterized defaults (Oracle, # SQLite). If this is the case, the individual schema backend # should implement prepare_default(). default_sql %= tuple(self.prepare_default(p) for p in params) @@ -431,9 +433,10 @@ class BaseDatabaseSchemaEditor: """Return the SQL to use in a GENERATED ALWAYS clause.""" expression_sql, params = field.generated_sql(self.connection) persistency_sql = "STORED" if field.db_persist else "VIRTUAL" - if params: + if self.connection.features.requires_literal_defaults: expression_sql = expression_sql % tuple(self.quote_value(p) for p in params) - return f"GENERATED ALWAYS AS ({expression_sql}) {persistency_sql}" + params = () + return f"GENERATED ALWAYS AS ({expression_sql}) {persistency_sql}", params @staticmethod def _effective_default(field): @@ -484,7 +487,7 @@ class BaseDatabaseSchemaEditor: """ sql, params = self.table_sql(model) # Prevent using [] as params, in the case a literal '%' is used in the - # definition. + # definition on backends that don't support parametrized DDL. self.execute(sql, params or None) if self.connection.features.supports_comments: @@ -747,7 +750,9 @@ class BaseDatabaseSchemaEditor: "column": self.quote_name(field.column), "definition": definition, } - self.execute(sql, params) + # Prevent using [] as params, in the case a literal '%' is used in the + # definition on backends that don't support parametrized DDL. + self.execute(sql, params or None) # Drop the default if we need to if ( field.db_default is NOT_PROVIDED diff --git a/django/db/backends/oracle/schema.py b/django/db/backends/oracle/schema.py index c8dd64650f..0d70522a2a 100644 --- a/django/db/backends/oracle/schema.py +++ b/django/db/backends/oracle/schema.py @@ -198,9 +198,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): return self.normalize_name(for_name + "_" + suffix) def prepare_default(self, value): - # Replace % with %% as %-formatting is applied in - # FormatStylePlaceholderCursor._fix_for_params(). - return self.quote_value(value).replace("%", "%%") + return self.quote_value(value) def _field_should_be_indexed(self, model, field): create_index = super()._field_should_be_indexed(model, field) diff --git a/docs/releases/5.0.4.txt b/docs/releases/5.0.4.txt index 6872d5998a..6b922b09c0 100644 --- a/docs/releases/5.0.4.txt +++ b/docs/releases/5.0.4.txt @@ -24,3 +24,7 @@ Bugfixes * Fixed a crash in Django 5.0 when performing queries involving table aliases and lookups on a ``GeneratedField`` of the aliased table (:ticket:`35344`). + +* Fixed a bug in Django 5.0 that caused a migration crash when adding a + ``GeneratedField`` relying on the ``__contains`` or ``__icontains`` + lookups or using a ``Value`` containing a ``"%"`` (:ticket:`35336`). diff --git a/tests/schema/tests.py b/tests/schema/tests.py index fff4982315..740cd5b0d0 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -54,7 +54,16 @@ from django.db.models import ( Value, ) from django.db.models.fields.json import KT, KeyTextTransform -from django.db.models.functions import Abs, Cast, Collate, Lower, Random, Round, Upper +from django.db.models.functions import ( + Abs, + Cast, + Collate, + Concat, + Lower, + Random, + Round, + Upper, +) from django.db.models.indexes import IndexExpression from django.db.transaction import TransactionManagementError, atomic from django.test import ( @@ -892,6 +901,39 @@ class SchemaTests(TransactionTestCase): with connection.schema_editor() as editor: editor.create_model(GeneratedFieldOutputFieldModel) + @isolate_apps("schema") + @skipUnlessDBFeature("supports_stored_generated_columns") + def test_add_generated_field_contains(self): + class GeneratedFieldContainsModel(Model): + text = TextField(default="foo") + generated = GeneratedField( + expression=Concat("text", Value("%")), + db_persist=True, + output_field=TextField(), + ) + + class Meta: + app_label = "schema" + + with connection.schema_editor() as editor: + editor.create_model(GeneratedFieldContainsModel) + + field = GeneratedField( + expression=Q(text__icontains="FOO"), + db_persist=True, + output_field=BooleanField(), + ) + field.contribute_to_class(GeneratedFieldContainsModel, "contains_foo") + + with connection.schema_editor() as editor: + editor.add_field(GeneratedFieldContainsModel, field) + + obj = GeneratedFieldContainsModel.objects.create() + obj.refresh_from_db() + self.assertEqual(obj.text, "foo") + self.assertEqual(obj.generated, "foo%") + self.assertIs(obj.contains_foo, True) + @isolate_apps("schema") def test_add_auto_field(self): class AddAutoFieldModel(Model):