1
0
mirror of https://github.com/django/django.git synced 2025-03-13 02:40:47 +00:00

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.
This commit is contained in:
Simon Charette 2024-04-02 16:33:31 -04:00 committed by GitHub
parent 5f18021640
commit 888b9042b3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 62 additions and 13 deletions

View File

@ -317,9 +317,9 @@ class BaseDatabaseSchemaEditor:
if default_value is not None: if default_value is not None:
column_default = "DEFAULT " + self._column_default_sql(field) column_default = "DEFAULT " + self._column_default_sql(field)
if self.connection.features.requires_literal_defaults: if self.connection.features.requires_literal_defaults:
# Some databases can't take defaults as a parameter (Oracle). # Some databases can't take defaults as a parameter
# If this is the case, the individual schema backend should # (Oracle, SQLite). If this is the case, the individual
# implement prepare_default(). # schema backend should implement prepare_default().
yield column_default % self.prepare_default(default_value) yield column_default % self.prepare_default(default_value)
else: else:
yield column_default yield column_default
@ -333,7 +333,9 @@ class BaseDatabaseSchemaEditor:
): ):
null = True null = True
if field.generated: 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: elif not null:
yield "NOT NULL" yield "NOT NULL"
elif not self.connection.features.implied_column_null: elif not self.connection.features.implied_column_null:
@ -420,7 +422,7 @@ class BaseDatabaseSchemaEditor:
compiler = query.get_compiler(connection=self.connection) compiler = query.get_compiler(connection=self.connection)
default_sql, params = compiler.compile(db_default) default_sql, params = compiler.compile(db_default)
if self.connection.features.requires_literal_defaults: 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 # SQLite). If this is the case, the individual schema backend
# should implement prepare_default(). # should implement prepare_default().
default_sql %= tuple(self.prepare_default(p) for p in params) 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.""" """Return the SQL to use in a GENERATED ALWAYS clause."""
expression_sql, params = field.generated_sql(self.connection) expression_sql, params = field.generated_sql(self.connection)
persistency_sql = "STORED" if field.db_persist else "VIRTUAL" 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) 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 @staticmethod
def _effective_default(field): def _effective_default(field):
@ -484,7 +487,7 @@ class BaseDatabaseSchemaEditor:
""" """
sql, params = self.table_sql(model) sql, params = self.table_sql(model)
# Prevent using [] as params, in the case a literal '%' is used in the # 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) self.execute(sql, params or None)
if self.connection.features.supports_comments: if self.connection.features.supports_comments:
@ -746,7 +749,9 @@ class BaseDatabaseSchemaEditor:
"column": self.quote_name(field.column), "column": self.quote_name(field.column),
"definition": definition, "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 # Drop the default if we need to
if ( if (
field.db_default is NOT_PROVIDED field.db_default is NOT_PROVIDED

View File

@ -198,9 +198,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
return self.normalize_name(for_name + "_" + suffix) return self.normalize_name(for_name + "_" + suffix)
def prepare_default(self, value): def prepare_default(self, value):
# Replace % with %% as %-formatting is applied in return self.quote_value(value)
# FormatStylePlaceholderCursor._fix_for_params().
return self.quote_value(value).replace("%", "%%")
def _field_should_be_indexed(self, model, field): def _field_should_be_indexed(self, model, field):
create_index = super()._field_should_be_indexed(model, field) create_index = super()._field_should_be_indexed(model, field)

View File

@ -24,3 +24,7 @@ Bugfixes
* Fixed a crash in Django 5.0 when performing queries involving table aliases * Fixed a crash in Django 5.0 when performing queries involving table aliases
and lookups on a ``GeneratedField`` of the aliased table (:ticket:`35344`). 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`).

View File

@ -54,7 +54,16 @@ from django.db.models import (
Value, Value,
) )
from django.db.models.fields.json import KT, KeyTextTransform 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.models.indexes import IndexExpression
from django.db.transaction import TransactionManagementError, atomic from django.db.transaction import TransactionManagementError, atomic
from django.test import TransactionTestCase, skipIfDBFeature, skipUnlessDBFeature from django.test import TransactionTestCase, skipIfDBFeature, skipUnlessDBFeature
@ -886,6 +895,39 @@ class SchemaTests(TransactionTestCase):
with connection.schema_editor() as editor: with connection.schema_editor() as editor:
editor.create_model(GeneratedFieldOutputFieldModel) 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") @isolate_apps("schema")
def test_add_auto_field(self): def test_add_auto_field(self):
class AddAutoFieldModel(Model): class AddAutoFieldModel(Model):