summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Charette <charette.s@gmail.com>2024-12-22 23:00:04 -0500
committerSarah Boyce <42296566+sarahboyce@users.noreply.github.com>2025-02-06 16:57:44 +0100
commit089deb82b9ac2d002af36fd36f288368cdac4b53 (patch)
tree5a9952ec4a9fa0ee73b233bc5551ac645543ea40
parentb13b8684a04d0bc1081104c5973c62c27dc673b0 (diff)
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.
-rw-r--r--django/db/models/lookups.py32
-rw-r--r--tests/expressions/tests.py17
-rw-r--r--tests/model_fields/test_jsonfield.py3
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,