FilteredRelation resolves its conditions too late which can result in unknown alias references at SQL compilation time

[Daniel Schaffer]

When using the Coalesce function as part of the condition of a FilteredRelation, the query fails with an "Unknown column" error if any of the fields referenced by Coalesce requires a JOIN. This appears to be due to the JOIN not actually getting added to the query.

            job_worker_preference=FilteredRelation(
                relation_name="company__worker_preferences",
                condition=Q(
                    company__worker_preferences__worker=Coalesce(F("worker"), F("worker_substitutions__worker")),
                    company__worker_preferences__company=F("company"),
                )
            ),
            is_allowed=Case(When(job_worker_preference__allow_assignments=True, then=1), default=0, output_field=BooleanField())

This can be worked around by creating a separate annotation for the result of the Coalesce function:

            actual_worker=Coalesce(F("worker"), F("worker_substitutions__worker")),
            job_worker_preference=FilteredRelation(
                relation_name="company__worker_preferences",
                condition=Q(
                    company__worker_preferences__worker=F("actual_worker"),
                    company__worker_preferences__company=F("company"),
                )
            ),
            is_allowed=Case(When(job_worker_preference__allow_assignments=True, then=1), default=0, output_field=BooleanField())

However, I suspect there may be an underlying issue with how JOINs are detected and added to a query when there are nested field references like this.

I've reproduced the issue in this repro: ​https://github.com/DanielSchaffer/django_filtered_relation_coalesce_repro.

django_filtered_relation_coalesce_repro/test/test_job_manager.py contains a failing test that reproduces the issue, and a passing test that demonstrates the workaround.

Here's the stringified representation of the query - note the missing JOIN to django_filtered_relation_coalesce_repro_workersubstitution, even though it's referenced in the COALESCE expression:

SELECT
  `django_filtered_relation_coalesce_repro_job`.`id`,
  `django_filtered_relation_coalesce_repro_job`.`company_id`,
  `django_filtered_relation_coalesce_repro_job`.`worker_id`,
  CASE WHEN job_worker_preference.`allow_assignments` THEN 1 ELSE 0 END AS `is_allowed`
FROM `django_filtered_relation_coalesce_repro_job`
INNER JOIN `django_filtered_relation_coalesce_repro_company`
ON (`django_filtered_relation_coalesce_repro_job`.`company_id` = `django_filtered_relation_coalesce_repro_company`.`id`)
LEFT OUTER JOIN `django_filtered_relation_coalesce_repro_workerpreference` job_worker_preference
ON (`django_filtered_relation_coalesce_repro_company`.`id` = job_worker_preference.`company_id` AND
    ((job_worker_preference.`company_id` = `django_filtered_relation_coalesce_repro_job`.`company_id` AND
      job_worker_preference.`worker_id` = COALESCE(`django_filtered_relation_coalesce_repro_job`.`worker_id`,
                                                   `django_filtered_relation_coalesce_repro_workersubstitution`.`worker_id`))))

[Simon Charette]

Thank you for your report, please provide a minimal set of models that reproduce the issue and the resulting traceback and query if possible.

That'll make the job of volunteers trying to validate your report way easier.

[Daniel Schaffer]

Sure, I've updated the ticket with a link to a repro repo

[Simon Charette]

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 ​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 ​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.

tests/filtered_relation/tests.py

@@ -685 +685 @@
             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

[Daniel Schaffer]

Thanks for that explanation Simon - I wouldn't have guessed this was quite that complicated an issue, but that all makes sense (and an interesting read to boot!).

[Martin Schmidt]

I have a similar issue with FilteredRelation and joins and traced it back to a django update from 4.0.3 to 4.0.6.
My guess is that this specific issue was introduced in https://code.djangoproject.com/ticket/33598

The provided workaround helped me a lot. Thanks!

[Mariusz Felisiak]

#33929 was closed as a duplicate. We should add tests for #33929 when resolving this issue.

[Mariusz Felisiak]

#34229 was closed as a duplicate. We should add tests for #34229 when resolving this issue.

[zhu]

The FilteredRelation cannot resolve itself too.

    def test_ref_self_in_condition(self):
        self.assertSequenceEqual(
            Author.objects.annotate(
                the_book=FilteredRelation(
                    "book",
                    condition=Q(
                        name=F('book__title'),
                        ),
                ),
            )
            .filter(
                the_book__isnull=False,
            ),
            [],
        )

[Francesco Panico]

Could this bug be resolved after fixing 34362? In the PR I encountered exactly this error

[Simon Charette]

Francesco, it's the other way around, fixing this bug is required to address #34362.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d660cee5:

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

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.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 1a13161:

Refs #33766 -- Removed unused Join.equals().

It's unused now that the specialized FilteredRelation.as_sql logic is
no more.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 83c9765:

Refs #33766 -- Removed sql.Query.build_filtered_relation_q().

It was a copy of sql.Query._add_q that avoided JOIN updates.