mirrors/django
1
0
Fork 0
mirror of https://github.com/django/django.git synced 2025-01-18 22:33:44 +00:00
Code Issues 32 Releases Wiki Activity

Fixed #33766 -- Resolved FilteredRelation.condition at referencing time.

Browse Source 
The previous implementation resolved condition at Join compilation time
which required introducing a specialized expression resolving mode to
alter the join reuse logic solely during that phase.

FilteredRelation.condition is now resolved when the relation is first
referenced which maintains the existing behavior while allowing the
removal of the specialized resolving mode and address an issue where
conditions couldn't spawn new joins.
This commit is contained in:
Simon Charette 2023-04-20 00:25:57 -04:00 committed by Mariusz Felisiak
parent 3fe0c609cf
commit d660cee5bc
4 changed files with 99 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
django/db/models/query_utils.py
View File

@ -403,8 +403,11 @@ class FilteredRelation:
self.alias = None
if not isinstance(condition, Q):
raise ValueError("condition argument must be a Q() instance.")
# .condition and .resolved_condition have to be stored independently
# as the former must remain unchanged for Join.__eq__ to remain stable
# and reusable even once their .filtered_relation are resolved.
self.condition = condition
self.path = []
self.resolved_condition = None
def __eq__(self, other):
if not isinstance(other, self.__class__):
@ -418,18 +421,22 @@ class FilteredRelation:
def clone(self):
clone = FilteredRelation(self.relation_name, condition=self.condition)
clone.alias = self.alias
clone.path = self.path[:]
if (resolved_condition := self.resolved_condition) is not None:
clone.resolved_condition = resolved_condition.clone()
return clone
def resolve_expression(self, *args, **kwargs):
"""
QuerySet.annotate() only accepts expression-like arguments
(with a resolve_expression() method).
"""
raise NotImplementedError("FilteredRelation.resolve_expression() is unused.")
def relabeled_clone(self, change_map):
clone = self.clone()
if resolved_condition := clone.resolved_condition:
clone.resolved_condition = resolved_condition.relabeled_clone(change_map)
return clone
def resolve_expression(self, query, reuse, *args, **kwargs):
clone = self.clone()
clone.resolved_condition = query.build_filtered_relation_q(
self.condition, reuse=reuse
)
return clone
def as_sql(self, compiler, connection):
# Resolve the condition in Join.filtered_relation.
query = compiler.query
where = query.build_filtered_relation_q(self.condition, reuse=set(self.path))
return compiler.compile(where)
return compiler.compile(self.resolved_condition)

5
django/db/models/sql/datastructures.py
View File

@ -154,10 +154,7 @@ class Join:
new_parent_alias = change_map.get(self.parent_alias, self.parent_alias)
new_table_alias = change_map.get(self.table_alias, self.table_alias)
if self.filtered_relation is not None:
filtered_relation = self.filtered_relation.clone()
filtered_relation.path = [
change_map.get(p, p) for p in self.filtered_relation.path
]
filtered_relation = self.filtered_relation.relabeled_clone(change_map)
else:
filtered_relation = None
return self.__class__(

70
django/db/models/sql/query.py
View File

@ -73,6 +73,19 @@ def get_children_from_q(q):
yield child
def rename_prefix_from_q(prefix, replacement, q):
return Q.create(
[
rename_prefix_from_q(prefix, replacement, c)
if isinstance(c, Node)
else (c[0].replace(prefix, replacement, 1), c[1])
for c in q.children
],
q.connector,
q.negated,
)
JoinInfo = namedtuple(
"JoinInfo",
("final_field", "targets", "opts", "joins", "path", "transform_function"),
@ -994,7 +1007,7 @@ class Query(BaseExpression):
"""
return len([1 for count in self.alias_refcount.values() if count])
def join(self, join, reuse=None, reuse_with_filtered_relation=False):
def join(self, join, reuse=None):
"""
Return an alias for the 'join', either reusing an existing alias for
that join or creating a new one. 'join' is either a base_table_class or
@ -1003,23 +1016,15 @@ class Query(BaseExpression):
The 'reuse' parameter can be either None which means all joins are
reusable, or it can be a set containing the aliases that can be reused.
The 'reuse_with_filtered_relation' parameter is used when computing
FilteredRelation instances.
A join is always created as LOUTER if the lhs alias is LOUTER to make
sure chains like t1 LOUTER t2 INNER t3 aren't generated. All new
joins are created as LOUTER if the join is nullable.
"""
if reuse_with_filtered_relation and reuse:
reuse_aliases = [
a for a, j in self.alias_map.items() if a in reuse and j.equals(join)
]
else:
reuse_aliases = [
a
for a, j in self.alias_map.items()
if (reuse is None or a in reuse) and j == join
]
reuse_aliases = [
a
for a, j in self.alias_map.items()
if (reuse is None or a in reuse) and j == join
]
if reuse_aliases:
if join.table_alias in reuse_aliases:
reuse_alias = join.table_alias
@ -1042,6 +1047,18 @@ class Query(BaseExpression):
join.join_type = join_type
join.table_alias = alias
self.alias_map[alias] = join
if filtered_relation := join.filtered_relation:
resolve_reuse = reuse
if resolve_reuse is not None:
resolve_reuse = set(reuse) | {alias}
joins_len = len(self.alias_map)
join.filtered_relation = filtered_relation.resolve_expression(
self, reuse=resolve_reuse
)
# Some joins were during expression resolving, they must be present
# before the one we just added.
if joins_len < len(self.alias_map):
self.alias_map[alias] = self.alias_map.pop(alias)
return alias
def join_parent_model(self, opts, model, alias, seen):
@ -1328,7 +1345,6 @@ class Query(BaseExpression):
can_reuse=None,
allow_joins=True,
split_subq=True,
reuse_with_filtered_relation=False,
check_filterable=True,
summarize=False,
):
@ -1353,9 +1369,6 @@ class Query(BaseExpression):
The 'can_reuse' is a set of reusable joins for multijoins.
If 'reuse_with_filtered_relation' is True, then only joins in can_reuse
will be reused.
The method will create a filter clause that can be added to the current
query. However, if the filter isn't added to the query then the caller
is responsible for unreffing the joins used.
@ -1417,7 +1430,6 @@ class Query(BaseExpression):
alias,
can_reuse=can_reuse,
allow_many=allow_many,
reuse_with_filtered_relation=reuse_with_filtered_relation,
)
# Prevent iterator from being consumed by check_related_objects()
@ -1575,7 +1587,6 @@ class Query(BaseExpression):
current_negated=current_negated,
allow_joins=True,
split_subq=False,
reuse_with_filtered_relation=True,
)
target_clause.add(child_clause, connector)
return target_clause
@ -1609,6 +1620,11 @@ class Query(BaseExpression):
"relations deeper than the relation_name (got %r for "
"%r)." % (lookup, filtered_relation.relation_name)
)
filtered_relation.condition = rename_prefix_from_q(
filtered_relation.relation_name,
alias,
filtered_relation.condition,
)
self._filtered_relations[filtered_relation.alias] = filtered_relation
def names_to_path(self, names, opts, allow_many=True, fail_on_missing=False):
@ -1734,7 +1750,6 @@ class Query(BaseExpression):
alias,
can_reuse=None,
allow_many=True,
reuse_with_filtered_relation=False,
):
"""
Compute the necessary table joins for the passage through the fields
@ -1747,9 +1762,6 @@ class Query(BaseExpression):
that can be reused. Note that non-reverse foreign keys are always
reusable when using setup_joins().
The 'reuse_with_filtered_relation' can be used to force 'can_reuse'
parameter and force the relation on the given connections.
If 'allow_many' is False, then any reverse foreign key seen will
generate a MultiJoin exception.
@ -1841,15 +1853,9 @@ class Query(BaseExpression):
nullable,
filtered_relation=filtered_relation,
)
reuse = can_reuse if join.m2m or reuse_with_filtered_relation else None
alias = self.join(
connection,
reuse=reuse,
reuse_with_filtered_relation=reuse_with_filtered_relation,
)
reuse = can_reuse if join.m2m else None
alias = self.join(connection, reuse=reuse)
joins.append(alias)
if filtered_relation:
filtered_relation.path = joins[:]
return JoinInfo(final_field, targets, opts, joins, path, final_transformer)
def trim_joins(self, targets, joins, path):

41
tests/filtered_relation/tests.py
View File

@ -793,6 +793,47 @@ class FilteredRelationAggregationTests(TestCase):
qs.annotate(total=Count("pk")).values("total"), [{"total": 1}]
)
def test_condition_spans_join(self):
self.assertSequenceEqual(
Book.objects.annotate(
contains_editor_author=FilteredRelation(
"author", condition=Q(author__name__icontains=F("editor__name"))
)
).filter(
contains_editor_author__isnull=False,
),
[self.book1],
)
def test_condition_spans_join_chained(self):
self.assertSequenceEqual(
Book.objects.annotate(
contains_editor_author=FilteredRelation(
"author", condition=Q(author__name__icontains=F("editor__name"))
),
contains_editor_author_ref=FilteredRelation(
"author",
condition=Q(author__name=F("contains_editor_author__name")),
),
).filter(
contains_editor_author_ref__isnull=False,
),
[self.book1],
)
def test_condition_self_ref(self):
self.assertSequenceEqual(
Book.objects.annotate(
contains_author=FilteredRelation(
"author",
condition=Q(title__icontains=F("author__name")),
)
).filter(
contains_author__isnull=False,
),
[self.book1],
)
class FilteredRelationAnalyticalAggregationTests(TestCase):
@classmethod