Browse Source

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.
Simon Charette 3 tháng trước cách đây
mục cha
commit
089deb82b9

+ 30 - 2
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)
 

+ 17 - 0
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):

+ 2 - 1
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,