Custom Join Conditions

317 views
Skip to first unread message

Josh Smeaton

unread,
Aug 28, 2017, 7:32:04 PM8/28/17
to Django developers (Contributions to Django itself)
There's currently a patch https://github.com/django/django/pull/7560 for ticket https://code.djangoproject.com/ticket/27332 that implements custom join conditions. It was nearly ready for merge for 1.11 but slipped. There are some merge conflicts the author still needs to sort out, but nothing too onerous.

Example:

    Author.objects
          .filtered_relation(
              'book', alias='book_alice',
              condition=Q(book__title__iexact='poem by alice')
          )

It has received very little attention and I'd like to get it on the radar for folks that would like to see this implemented, and have some time to do review. I'll be trying to get some review done in the next few days depending on my schedule.

One minor concern I did have was with the naming of the `filtered_relation` queryset method. I was leaning towards `filter_related` to better align with select_related and prefetch_related but I'm not sure that's better, or if it gives the wrong idea about what the method does. It might also be worth renaming the `alias` kwarg to `to_attr` to align with prefetch_related API. (Even if you don't have time to review, bikeshedding names seems to be popular 'round these parts :) ).

Adam Johnson

unread,
Sep 2, 2017, 6:00:43 AM9/2/17
to django-d...@googlegroups.com
I think filter_related is a good name, it's not that confusing that it sounds like select_related / prefetch_related. The current name is a bit weird because it's an adj + noun, it doesn't really say how it modifies the QuerySet, it sounds more declarative.

to_attr is also better, as it aligns with prefetch_related.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3f46645a-3535-42e1-9d3c-ef8e5d5d3694%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
Adam

Shai Berger

unread,
Sep 2, 2017, 1:27:30 PM9/2/17
to django-d...@googlegroups.com
I think that giving this funcionality names that follow the patterns of
select_related() or prefetch_related() is bad, because although it is very
close to them in subdomains (dealing with relations), its functionality is
much closer to annotate(). In particular, alias is better than to_attr because
the alias here (as in annotate()) can be used in conditions in further filter()
calls, while a prefetched to_attr name cannot.

The one sore point I see in the name 'filtered_relation' is that it's a noun-
phrase, where similar methods are verbs (filter, exclude, annotate, defer) or
verb phrases (select_related). I would prefer a name like
add_filtered_relation, but that may seem cumbersome.

Actually, thinking more about this, a better interface may be

Author.objects.annotate_fitered (
book_alice = ('book', Q(book__title__iexact='poem by alice')),
)

or even

Author.objects.annotate (
book_alice =
FilteredRelation('book', book__title__iexact='poem by alice')
)

That is, overload annotate to accept, as target, a special object with
initializer

class FilteredRelation(...):
def __init__(relation_name, *q_conditions, **conditions)

My 2 cents,
Shai.

On Saturday 02 September 2017 13:00:08 Adam Johnson wrote:
> I think filter_related is a good name, it's not that confusing that it
> sounds like select_related / prefetch_related. The current name is a bit
> weird because it's an adj + noun, it doesn't really say how it *modifies*
> > email to django-develop...@googlegroups.com.
> > To post to this group, send email to django-d...@googlegroups.com.
> > Visit this group at https://groups.google.com/group/django-developers.
> > To view this discussion on the web visit https://groups.google.com/d/
> > msgid/django-developers/3f46645a-3535-42e1-9d3c-
> > ef8e5d5d3694%40googlegroups.com
> > <https://groups.google.com/d/msgid/django-developers/3f46645a-3535-42e1-9
> > d3c-ef8e5d5d3694%40googlegroups.com?utm_medium=email&utm_source=footer> .

Anssi Kääriäinen

unread,
Sep 4, 2017, 5:25:25 AM9/4/17
to Django developers (Contributions to Django itself)
I really like the .annotate() version. The way this feature works with .annotate() is easy to understand if you understand how existing .annotate() functions work. But, if we go with this approach, it might look a bit strange if we don't do an automatic select_related(). On the other hand, there are cases where you definitely don't want to do a select_related(), like if the relation annotation is only used in further aggregation. So, maybe there should be opt-out, or maybe we just don't do any automatic select_related for this case.

I do like the .annotate() version for other reasons, too. There are variants of this feature which would work nicely with the annotate style, that is either annotation querysets for objects that do *not* have a relation to the original queryset, or annotation raw SQL directly.

- Anssi
PS. It seems the Google Groups UI acts a bit strange for me today. I hope this post does arrive to the group in a readable format...

Adam Johnson

unread,
Sep 4, 2017, 7:31:01 AM9/4/17
to django-d...@googlegroups.com
The annotate(foo=FilteredRelation(...)) API LGTM too, and yes I agree on the rationale on alias over to_attr.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
Adam

Nicolas Delaby

unread,
Sep 5, 2017, 4:43:45 AM9/5/17
to django-d...@googlegroups.com
It seems we have a consensus.
I'll be glad to push that change.

I just have a question, from where do you want users importing the class FilteredRelation ?
From its current location ?

from django.db.models.query import FilteredRelation

or from its parent module ?

from django.db.models import FilteredRelation

as it seems to be the django way.

Best,
Nicolas

On 04/09/2017 13:30, Adam Johnson wrote:
> The annotate(foo=FilteredRelation(...)) API LGTM too, and yes I agree on the rationale on alias over to_attr.
>
> On 4 September 2017 at 10:25, Anssi Kääriäinen <akaa...@gmail.com <mailto:akaa...@gmail.com>> wrote:
>
> I really like the .annotate() version. The way this feature works with .annotate() is easy to understand if you understand how existing
> .annotate() functions work. But, if we go with this approach, it might look a bit strange if we don't do an automatic select_related(). On the
> other hand, there are cases where you definitely don't want to do a select_related(), like if the relation annotation is only used in further
> aggregation. So, maybe there should be opt-out, or maybe we just don't do any automatic select_related for this case.
>
> I do like the .annotate() version for other reasons, too. There are variants of this feature which would work nicely with the annotate style, that
> is either annotation querysets for objects that do *not* have a relation to the original queryset, or annotation raw SQL directly.
>
> - Anssi
> PS. It seems the Google Groups UI acts a bit strange for me today. I hope this post does arrive to the group in a readable format...
>
> --
> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com
> <mailto:django-developers%2Bunsu...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com <mailto:django-d...@googlegroups.com>.
> Visit this group at https://groups.google.com/group/django-developers <https://groups.google.com/group/django-developers>.
> <https://groups.google.com/d/msgid/django-developers/4ceb0715-bd6b-4860-a6e6-77dc9ecabae6%40googlegroups.com>.
> For more options, visit https://groups.google.com/d/optout <https://groups.google.com/d/optout>.
>
>
>
>
> --
> Adam
>
> --
> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com <mailto:django-d...@googlegroups.com>.
> Visit this group at https://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAMyDDM0n5K9nLz1V1kWgEiHnRiOX4V6ai_TpFp0ahoXV5GDbVQ%40mail.gmail.com
> <https://groups.google.com/d/msgid/django-developers/CAMyDDM0n5K9nLz1V1kWgEiHnRiOX4V6ai_TpFp0ahoXV5GDbVQ%40mail.gmail.com?utm_medium=email&utm_source=footer>.

Adam Johnson

unread,
Sep 5, 2017, 5:10:58 AM9/5/17
to django-d...@googlegroups.com
from django.db.models import FilteredRelation is what I'd expect

On 5 September 2017 at 09:44, Nicolas Delaby <tic...@free.fr> wrote:
It seems we have a consensus.
I'll be glad to push that change.

I just have a question, from where do you want users importing the class FilteredRelation ?
From its current location ?

  from django.db.models.query import FilteredRelation

or from its parent module ?

  from django.db.models import FilteredRelation

as it seems to be the django way.

Best,
Nicolas

On 04/09/2017 13:30, Adam Johnson wrote:
> The annotate(foo=FilteredRelation(...)) API LGTM too, and yes I agree on the rationale on alias over to_attr.
>
> On 4 September 2017 at 10:25, Anssi Kääriäinen <akaa...@gmail.com <mailto:akaa...@gmail.com>> wrote:
>
>     I really like the .annotate() version. The way this feature works with .annotate() is easy to understand if you understand how existing
>     .annotate() functions work. But, if we go with this approach, it might look a bit strange if we don't do an automatic select_related(). On the
>     other hand, there are cases where you definitely don't want to do a select_related(), like if the relation annotation is only used in further
>     aggregation. So, maybe there should be opt-out, or maybe we just don't do any automatic select_related for this case.
>
>     I do like the .annotate() version for other reasons, too. There are variants of this feature which would work nicely with the annotate style, that
>     is either annotation querysets for objects that do *not* have a relation to the original queryset, or annotation raw SQL directly.
>
>      - Anssi
>     PS. It seems the Google Groups UI acts a bit strange for me today. I hope this post does arrive to the group in a readable format...
>
>     --
>     You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
>     To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com
>     <mailto:django-developers%2Bunsu...@googlegroups.com>.
>     To post to this group, send email to django-developers@googlegroups.com <mailto:django-developers@googlegroups.com>.
>
> --
> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com
> <mailto:django-developers+unsub...@googlegroups.com>.
> To post to this group, send email to django-developers@googlegroups.com <mailto:django-developers@googlegroups.com>.
> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
Adam

Michael Manfre

unread,
Sep 5, 2017, 9:04:27 AM9/5/17
to django-d...@googlegroups.com
On Tue, Sep 5, 2017 at 5:10 AM Adam Johnson <m...@adamj.eu> wrote:
from django.db.models import FilteredRelation is what I'd expect

This is what I'd expect too.
 
On 5 September 2017 at 09:44, Nicolas Delaby <tic...@free.fr> wrote:
It seems we have a consensus.
I'll be glad to push that change.

I just have a question, from where do you want users importing the class FilteredRelation ?
From its current location ?

  from django.db.models.query import FilteredRelation

or from its parent module ?

  from django.db.models import FilteredRelation

as it seems to be the django way.

Best,
Nicolas

On 04/09/2017 13:30, Adam Johnson wrote:
> The annotate(foo=FilteredRelation(...)) API LGTM too, and yes I agree on the rationale on alias over to_attr.
>
> On 4 September 2017 at 10:25, Anssi Kääriäinen <akaa...@gmail.com <mailto:akaa...@gmail.com>> wrote:
>
>     I really like the .annotate() version. The way this feature works with .annotate() is easy to understand if you understand how existing
>     .annotate() functions work. But, if we go with this approach, it might look a bit strange if we don't do an automatic select_related(). On the
>     other hand, there are cases where you definitely don't want to do a select_related(), like if the relation annotation is only used in further
>     aggregation. So, maybe there should be opt-out, or maybe we just don't do any automatic select_related for this case.
>
>     I do like the .annotate() version for other reasons, too. There are variants of this feature which would work nicely with the annotate style, that
>     is either annotation querysets for objects that do *not* have a relation to the original queryset, or annotation raw SQL directly.
>
>      - Anssi
>     PS. It seems the Google Groups UI acts a bit strange for me today. I hope this post does arrive to the group in a readable format...
>
>     --
>     You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
>     To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com
>     <mailto:django-developers%2Bunsu...@googlegroups.com>.
>     To post to this group, send email to django-d...@googlegroups.com <mailto:django-d...@googlegroups.com>.
>
> --
> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com
> <mailto:django-develop...@googlegroups.com>.
> To post to this group, send email to django-d...@googlegroups.com <mailto:django-d...@googlegroups.com>.
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.



--
Adam

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages