From 5f89da0837009debda998af30f280d7075fb1a4d Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Tue, 12 Dec 2023 05:39:11 +0100 Subject: [PATCH] [5.0.x] Fixed #35018 -- Fixed migrations crash on GeneratedField with BooleanField as output_field on Oracle < 23c. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks Václav Řehák for the report. Regression in f333e3513e8bdf5ffeb6eeb63021c230082e6f95. Backport of fcf95e592774a6ededec35481a2061474d467a2b from main. --- django/db/backends/oracle/base.py | 39 +++++++++++++++++++-------- django/db/backends/oracle/features.py | 8 +++++- django/db/backends/oracle/utils.py | 1 + django/db/models/expressions.py | 11 +++++--- django/db/models/fields/generated.py | 8 +++++- docs/releases/5.0.1.txt | 4 +++ tests/lookup/tests.py | 1 - tests/schema/tests.py | 21 +++++++++++++++ 8 files changed, 75 insertions(+), 18 deletions(-) diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index 803c3a5baf..26341c702f 100644 --- a/django/db/backends/oracle/base.py +++ b/django/db/backends/oracle/base.py @@ -300,7 +300,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): @async_unsafe def create_cursor(self, name=None): - return FormatStylePlaceholderCursor(self.connection) + return FormatStylePlaceholderCursor(self.connection, self) def _commit(self): if self.connection is not None: @@ -365,11 +365,15 @@ class OracleParam: param = Oracle_datetime.from_datetime(param) string_size = 0 - # Oracle doesn't recognize True and False correctly. - if param is True: - param = 1 - elif param is False: - param = 0 + has_boolean_data_type = ( + cursor.database.features.supports_boolean_expr_in_select_clause + ) + if not has_boolean_data_type: + # Oracle < 23c doesn't recognize True and False correctly. + if param is True: + param = 1 + elif param is False: + param = 0 if hasattr(param, "bind_parameter"): self.force_bytes = param.bind_parameter(cursor) elif isinstance(param, (Database.Binary, datetime.timedelta)): @@ -389,6 +393,8 @@ class OracleParam: self.input_size = Database.CLOB elif isinstance(param, datetime.datetime): self.input_size = Database.TIMESTAMP + elif has_boolean_data_type and isinstance(param, bool): + self.input_size = Database.DB_TYPE_BOOLEAN else: self.input_size = None @@ -426,9 +432,10 @@ class FormatStylePlaceholderCursor: charset = "utf-8" - def __init__(self, connection): + def __init__(self, connection, database): self.cursor = connection.cursor() self.cursor.outputtypehandler = self._output_type_handler + self.database = database @staticmethod def _output_number_converter(value): @@ -528,14 +535,24 @@ class FormatStylePlaceholderCursor: # values. It can be used only in single query execute() because # executemany() shares the formatted query with each of the params # list. e.g. for input params = [0.75, 2, 0.75, 'sth', 0.75] - # params_dict = {0.75: ':arg0', 2: ':arg1', 'sth': ':arg2'} + # params_dict = { + # (float, 0.75): ':arg0', + # (int, 2): ':arg1', + # (str, 'sth'): ':arg2', + # } # args = [':arg0', ':arg1', ':arg0', ':arg2', ':arg0'] # params = {':arg0': 0.75, ':arg1': 2, ':arg2': 'sth'} + # The type of parameters in param_types keys is necessary to avoid + # unifying 0/1 with False/True. + param_types = [(type(param), param) for param in params] params_dict = { - param: ":arg%d" % i for i, param in enumerate(dict.fromkeys(params)) + param_type: ":arg%d" % i + for i, param_type in enumerate(dict.fromkeys(param_types)) + } + args = [params_dict[param_type] for param_type in param_types] + params = { + placeholder: param for (_, param), placeholder in params_dict.items() } - args = [params_dict[param] for param in params] - params = {value: key for key, value in params_dict.items()} query %= tuple(args) else: # Handle params as sequence diff --git a/django/db/backends/oracle/features.py b/django/db/backends/oracle/features.py index 8a02e098d6..ee1cfaa0d9 100644 --- a/django/db/backends/oracle/features.py +++ b/django/db/backends/oracle/features.py @@ -76,7 +76,6 @@ class DatabaseFeatures(BaseDatabaseFeatures): supports_slicing_ordering_in_compound = True requires_compound_order_by_subquery = True allows_multiple_constraints_on_same_fields = False - supports_boolean_expr_in_select_clause = False supports_comparing_boolean_expr = False supports_json_field_contains = False supports_collation_on_textfield = False @@ -119,6 +118,9 @@ class DatabaseFeatures(BaseDatabaseFeatures): "Oracle doesn't support comparing NCLOB to NUMBER.": { "generic_relations_regress.tests.GenericRelationTests.test_textlink_filter", }, + "Oracle doesn't support casting filters to NUMBER.": { + "lookup.tests.LookupQueryingTests.test_aggregate_combined_lookup", + }, } django_test_expected_failures = { # A bug in Django/oracledb with respect to string handling (#23843). @@ -166,3 +168,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): @cached_property def supports_primitives_in_json_field(self): return self.connection.oracle_version >= (21,) + + @cached_property + def supports_boolean_expr_in_select_clause(self): + return self.connection.oracle_version >= (23,) diff --git a/django/db/backends/oracle/utils.py b/django/db/backends/oracle/utils.py index 930b8e0918..28be58c73d 100644 --- a/django/db/backends/oracle/utils.py +++ b/django/db/backends/oracle/utils.py @@ -21,6 +21,7 @@ class InsertVar: "PositiveBigIntegerField": int, "PositiveSmallIntegerField": int, "PositiveIntegerField": int, + "BooleanField": int, "FloatField": Database.NATIVE_FLOAT, "DateTimeField": Database.TIMESTAMP, "DateField": Database.Date, diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index b162da513d..b3b66f0c21 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -1702,10 +1702,13 @@ class OrderBy(Expression): return (template % placeholders).rstrip(), params def as_oracle(self, compiler, connection): - # Oracle doesn't allow ORDER BY EXISTS() or filters unless it's wrapped - # in a CASE WHEN. - if connection.ops.conditional_expression_supported_in_where_clause( - self.expression + # Oracle < 23c doesn't allow ORDER BY EXISTS() or filters unless it's + # wrapped in a CASE WHEN. + if ( + not connection.features.supports_boolean_expr_in_select_clause + and connection.ops.conditional_expression_supported_in_where_clause( + self.expression + ) ): copy = self.copy() copy.expression = Case( diff --git a/django/db/models/fields/generated.py b/django/db/models/fields/generated.py index 95d19582de..257feeeba2 100644 --- a/django/db/models/fields/generated.py +++ b/django/db/models/fields/generated.py @@ -58,7 +58,13 @@ class GeneratedField(Field): resolved_expression = self.expression.resolve_expression( self._query, allow_joins=False ) - return compiler.compile(resolved_expression) + sql, params = compiler.compile(resolved_expression) + if ( + getattr(self.expression, "conditional", False) + and not connection.features.supports_boolean_expr_in_select_clause + ): + sql = f"CASE WHEN {sql} THEN 1 ELSE 0 END" + return sql, params def check(self, **kwargs): databases = kwargs.get("databases") or [] diff --git a/docs/releases/5.0.1.txt b/docs/releases/5.0.1.txt index 6b0b6de66c..2e720e57f0 100644 --- a/docs/releases/5.0.1.txt +++ b/docs/releases/5.0.1.txt @@ -20,3 +20,7 @@ Bugfixes * Fixed a regression in Django 5.0 that caused a crash of ``Model.save()`` for models with both ``GeneratedField`` and ``ForeignKey`` fields (:ticket:`35019`). + +* Fixed a bug in Django 5.0 that caused a migration crash on Oracle < 23c when + adding a ``GeneratedField`` with ``output_field=BooleanField`` + (:ticket:`35018`). diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py index 8af459ccd7..a198a13b62 100644 --- a/tests/lookup/tests.py +++ b/tests/lookup/tests.py @@ -1522,7 +1522,6 @@ class LookupQueryingTests(TestCase): qs = Season.objects.order_by(LessThan(F("year"), 1910), F("year")) self.assertSequenceEqual(qs, [self.s1, self.s3, self.s2]) - @skipUnlessDBFeature("supports_boolean_expr_in_select_clause") def test_aggregate_combined_lookup(self): expression = Cast(GreaterThan(F("year"), 1900), models.IntegerField()) qs = Season.objects.aggregate(modern=models.Sum(expression)) diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 046097366c..36c6b5e4bf 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -852,6 +852,27 @@ class SchemaTests(TransactionTestCase): False, ) + @isolate_apps("schema") + @skipUnlessDBFeature("supports_virtual_generated_columns") + def test_add_generated_boolean_field(self): + class GeneratedBooleanFieldModel(Model): + value = IntegerField(null=True) + has_value = GeneratedField( + expression=Q(value__isnull=False), + output_field=BooleanField(), + db_persist=False, + ) + + class Meta: + app_label = "schema" + + with connection.schema_editor() as editor: + editor.create_model(GeneratedBooleanFieldModel) + obj = GeneratedBooleanFieldModel.objects.create() + self.assertIs(obj.has_value, False) + obj = GeneratedBooleanFieldModel.objects.create(value=1) + self.assertIs(obj.has_value, True) + @isolate_apps("schema") @skipUnlessDBFeature("supports_stored_generated_columns") def test_add_generated_field(self):