Support nested relations in FilteredRelation's condition

[Thomas Riccardi]

As the documentation explains, FilteredRelation's condition do not support nested relations:

    >>> Restaurant.objects.annotate(
    ...    pizzas_with_toppings_startswith_n=FilteredRelation(
    ...        'pizzas__toppings',
    ...        condition=Q(pizzas__toppings__name__startswith='n'),
    ...    ),
    ... )
    Traceback (most recent call last):
    ...
    ValueError: FilteredRelation's condition doesn't support nested relations (got 'pizzas__toppings__name__startswith').

It would be great to support nested relations in FilteredRelation's condition (I encountered this limitation multiple times recently).

[Tim Graham]

Nicolas, do you remember the reason for the restriction?

[Nicolas Delaby]

I simply couldn't find a way to make it work, I don't remember exactly why and I didn't have real incitement to do it at that time.
The explicit ValueError is more a gate keeper preventing users to follow a path we know will fail cryptically.
My suggestion would be start writing some tests, removing the ValueError and see how it goes.
My priority at that time was to land the feature even with partial support. Now that a big chunk is there, we can more easily improve the support so nested relations is supported.
Unfortunately I'm at the moment away from django stack, so I Invite you, Thomas , to give it a try, if it is important for you. You might go further than I could.

[Simon Charette]

If I remember correctly there was some weirdness when dealing with exclusion of such nested filtered relations.

[saber solooki]

Are you support this feature in next version or not? thank's in advance

[Simon Charette]

#30349 addressed an issue when using exclude with FilteredRelation that might explain the ValueError raised when trying to add support for nested relations.

[Alex Scott]

Noumbissi, just wondering if you're working on this? And if so, how it's progressing?

This problem (of needing to join to a subquery or add additional conditions to a join that is nested more than once) seems to come up quite a bit. I've posted my own question on StackOverflow:
​https://stackoverflow.com/questions/58895627/django-queryset-with-aggregation-and-filtered-left-join-multiple-levels-deep

In the absence of this feature, it'd be nice to have someone intimately familiar with the ORM propose how they would tackle this as I haven't seen a "this feature is not yet available, but as a workaround, try this instead..". Would be very helpful.

[Matt Ferrante]

Noumbissi, Alex Scott, Nicolas Delaby
I've opened a PR for supporting Nested FilteredRelations in 2.2
I can also support this for 3.0, but I made the PR for 2.2 because I'm using 2.2. Happy to open the PR for 3.0 as well.

Feedback is appreciated, I included comments in other places I had examined for making the change.

​https://github.com/django/django/pull/12288

[Mariusz Felisiak]

Matt, we don't accept new features to the Django 2.2 and 3.0. Please create PR against master branch.

[Matt Ferrante]

Thanks for the heads-up felixxm. Opened one against master:
​https://github.com/django/django/pull/12293

I'll work through the test failures.

[Matt Ferrante]

Got the tests passing in ​https://github.com/django/django/pull/12333

[Mariusz Felisiak]

​PR

[Matt Ferrante]

felixxm, I've updated the PR, can you take another look? Thanks!
​https://github.com/django/django/pull/12333

[Mariusz Felisiak]

Matt, Thanks, you don't need to ping me, if PR is ready for another review just uncheck "Patch needs improvement".

[Matt Ferrante]

Patch available here, rebased and it's good to go!
​https://github.com/django/django/pull/12333

[Matt Ferrante]

felixxm, should I be doing something else to get this another review?

[Mariusz Felisiak]

Matt, you can ask any friend to review it. I'm sorry but you must be patient we have many PRs in a review queue.

[Matt Ferrante]

I've been using a branch with this patch for nested FilteredRelations in production for 4 months without issues. It works really well and added a lot of flexibility and allowed for performance optimizations which helped with our analytical query speed. Hoping this gets approved and changes to "Ready for checkin" soon!

[Simon Charette]

I think the proposed patch is ready to be merged. Looks like current implementation of FilteredRelation(condition=~Q) might do weird things under some circumstances but that seems like an artifact of how exclusions against multi-valued relationship is currently implemented.

[Mariusz Felisiak]

​PR with final edits

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7d6916e:

Fixed #29789 -- Added support for nested relations to FilteredRelation.

[Mariusz Felisiak <felisiak.mariusz@…>]

In e4a5527d:

Refs #29789 -- Added more tests for FilteredRelation with condition outside of relation name.