From 089deb82b9ac2d002af36fd36f288368cdac4b53 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sun, 22 Dec 2024 23:00:04 -0500 Subject: [PATCH] Fixed #36025 -- Fixed re-aliasing of iterable (in/range) lookups rhs. In order for Expression.relabeled_clone to work appropriately its get_source_expressions method must return all resolvable which wasn't the case for Lookup when its right-hand-side is "direct" (not a compilable). While refs #22288 added support for non-literals iterable right-hand-side lookups it predated the subclassing of Lookup(Expression) refs #27021 which could have been an opportunity to ensure right-hand-sides are always resolvable (ValueList and ExpressionList). Addressing all edge case with non-resolvable right-hand-sides would require a significant refactor and deprecation of some parts of the Lookup interface so this patch only focuses on FieldGetDbPrepValueIterableMixin (In and Range lookups) by making sure that a right-hand-side containing resolvables are dealt with appropriately during the resolving phase. Thanks Aashay Amballi for the report. --- django/db/models/lookups.py | 32 ++++++++++++++++++++++++++-- tests/expressions/tests.py | 17 +++++++++++++++ tests/model_fields/test_jsonfield.py | 3 ++- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index 860488794c..0aaa3ac6a8 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -2,7 +2,15 @@ import itertools import math from django.core.exceptions import EmptyResultSet, FullResultSet -from django.db.models.expressions import Case, ColPairs, Expression, Func, Value, When +from django.db.models.expressions import ( + Case, + ColPairs, + Expression, + ExpressionList, + Func, + Value, + When, +) from django.db.models.fields import ( BooleanField, CharField, @@ -279,12 +287,13 @@ class FieldGetDbPrepValueIterableMixin(FieldGetDbPrepValueMixin): def get_prep_lookup(self): if hasattr(self.rhs, "resolve_expression"): return self.rhs + contains_expr = False prepared_values = [] for rhs_value in self.rhs: if hasattr(rhs_value, "resolve_expression"): # An expression will be handled by the database but can coexist # alongside real values. - pass + contains_expr = True elif ( self.prepare_rhs and hasattr(self.lhs, "output_field") @@ -292,6 +301,19 @@ class FieldGetDbPrepValueIterableMixin(FieldGetDbPrepValueMixin): ): rhs_value = self.lhs.output_field.get_prep_value(rhs_value) prepared_values.append(rhs_value) + if contains_expr: + return ExpressionList( + *[ + # Expression defaults `str` to field references while + # lookups default them to literal values. + ( + Value(prep_value, self.lhs.output_field) + if isinstance(prep_value, str) + else prep_value + ) + for prep_value in prepared_values + ] + ) return prepared_values def process_rhs(self, compiler, connection): @@ -299,6 +321,12 @@ class FieldGetDbPrepValueIterableMixin(FieldGetDbPrepValueMixin): # rhs should be an iterable of values. Use batch_process_rhs() # to prepare/transform those values. return self.batch_process_rhs(compiler, connection) + elif isinstance(self.rhs, ExpressionList): + # rhs contains at least one expression. Unwrap them and delegate + # to batch_process_rhs() to prepare/transform those values. + copy = self.copy() + copy.rhs = self.rhs.get_source_expressions() + return copy.process_rhs(compiler, connection) else: return super().process_rhs(compiler, connection) diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index d36107b358..cfa33b6f45 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -1276,6 +1276,23 @@ class IterableLookupInnerExpressionsTests(TestCase): queryset = Result.objects.filter(**{lookup: within_experiment_time}) self.assertSequenceEqual(queryset, [r1]) + def test_relabeled_clone_rhs(self): + Number.objects.bulk_create([Number(integer=1), Number(integer=2)]) + self.assertIs( + Number.objects.filter( + # Ensure iterable of expressions are properly re-labelled on + # subquery pushdown. If the inner query __range right-hand-side + # members are not relabelled they will point at the outer query + # alias and this test will fail. + Exists( + Number.objects.exclude(pk=OuterRef("pk")).filter( + integer__range=(F("integer"), F("integer")) + ) + ) + ).exists(), + True, + ) + class FTests(SimpleTestCase): def test_deepcopy(self): diff --git a/tests/model_fields/test_jsonfield.py b/tests/model_fields/test_jsonfield.py index 09f95ce69f..5a9cf9ad7a 100644 --- a/tests/model_fields/test_jsonfield.py +++ b/tests/model_fields/test_jsonfield.py @@ -13,6 +13,7 @@ from django.db import ( OperationalError, connection, models, + transaction, ) from django.db.models import ( Count, @@ -974,7 +975,7 @@ class TestQuerying(TestCase): ("value__i__in", [False, "foo"], [self.objs[4]]), ] for lookup, value, expected in tests: - with self.subTest(lookup=lookup, value=value): + with self.subTest(lookup=lookup, value=value), transaction.atomic(): self.assertCountEqual( NullableJSONModel.objects.filter(**{lookup: value}), expected,