#33766: FilteredRelation doesn't resolve its conditions resulting in unknown
alias
references at SQL compilation time
-------------------------------------+-------------------------------------
Reporter: Daniel Schaffer | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: filteredrelation | Triage Stage: Accepted
coalesce |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):
* stage: Unreviewed => Accepted
Comment:
`FilteredRelation` annotations are implemented in a somewhat awkward way
in the sense that they are accepted by `Queryset.annotate` even they are
not truly resolvable.
From my understanding the reason behind that is to somewhat mimic what
`Queryset.alias` does for annotations to avoid incurring an extraneous
`JOIN` if the filtered relation end up not being referenced in the final
queryset composition.
This is achieved by having `Queryset.annotate` take an entirely different
route when provided a `FilteredRelation` which ''defers'' expression
resolving by stashing the instance and its alias in
`Query._filtered_relations`. This stash is then later used to create an
actual filtered join when `Query.names_to_path` detects a reference to a
previously stashed ''name'' in `._filtered_relations`.
What the actual implementation of `FilteredRelation` failed to account for
when making the latter rely on a special JIT resolving instead of using
the usual `resolve_expression` path is that `.conditions` right-hand-sides
are allowed to span joins (as in the reporter's case here with
`F("worker_substitutions__worker")`) and thus must be resolved before the
query is passed to the compiler. This is problematic because the
`.condition` of `FilteredRelation` is ''resolved'' via the
`SQLCompiler.compile -> .get_from_clause -> Join.as_sql ->
FilteredRelation.as_sql` chain but at the time `FilteredRelation.as_sql`
calls `compiler.query.build_filtered_relation_q`, which happens to augment
`query.alias_map` with the proper joins associated with the right-hand-
sides, it's already too late because the list of aliases to be considered
for compilation in `get_from_clause`
[https://github.com/django/django/blob/49b470b9187b6be60e573fed08c8f4a87f133750/django/db/models/sql/compiler.py#L960
is already fixed].
I don't have all the details at hand here but I wonder why we chose to go
this way instead of immediately creating the `alias_map` entry with a
corresponding `alias_refcount` of `0` which would have resulted in the
joins being referenceable but
[https://github.com/django/django/blob/49b470b9187b6be60e573fed08c8f4a87f133750/django/db/models/sql/compiler.py#L961-L962
eventually pruned if unused]. We have the machinery in place to track
dependency chains between joins so it would only have been a matter of
incrementing the transitive chain of joins in `names_to_path` when the
filtered relation was referenced and we should have been set.
I didn't have time to commit in exploring this route, which seems like a
good way of cleaning all the filtered relation logic, but here's at least
a small patch reproduces the issue in our current test suite.
{{{#!diff
diff --git a/tests/filtered_relation/tests.py
b/tests/filtered_relation/tests.py
index 7d77e31b51..825d150830 100644
--- a/tests/filtered_relation/tests.py
+++ b/tests/filtered_relation/tests.py
@@ -685,6 +685,20 @@ def test_eq(self):
FilteredRelation("book", condition=Q(book__title="b")),
mock.ANY
)
+ def test_condition_spans_join(self):
+ self.assertSequenceEqual(
+ Book.objects.annotate(
+ contains_editor_author=FilteredRelation(
+ "author",
condition=Q(author__name__contains=F("editor__name"))
+ )
+ )
+ .filter(
+ contains_editor_author__isnull=False,
+ )
+ .order_by("id"),
+ [cls.book1, cls.book4],
+ )
+
class FilteredRelationAggregationTests(TestCase):
@classmethod
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33766#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/01070181321f888a-ee1a0b61-3767-4562-b43f-83507bfc45dd-000000%40eu-central-1.amazonses.com.
