Feature proposal: Q.push(...) and QuerySet.filter(relfield=Q(...))

53 views
Skip to first unread message

Johannes Dollinger

unread,
Sep 30, 2011, 5:37:35 PM9/30/11
to django-d...@googlegroups.com
The aim of this proposal is to reuse Q objects for models that are related through FK or M2M fields.
A simplified example would be a Q object like

>>> is_blue = Q(blue=True)
>>> Thing.objects.filter(is_blue)

that should be reused to filter the owners of things:

>>> User.objects.filter(things__blue=True).

Currently, the only way to reuse the Q object would be subquery:

>>> User.objects.filter(things__in=Thing.objects.filter(is_blue)).

I therefore propose the following API:

>>> Q(lookup=value).push('relfield')

which would be equivalent to

>>> Q(relfield__lookup=value)

and

>>> qs.filter(relfield=Q(lookup=value))

which would be equivalent to

>>> qs.filter(Q(lookup=value).push('relfield')).

We then could write

>>> User.objects.filter(things=is_blue)

which is shorter and produces more natural SQL than the subquery solution.

Thoughs? Suggestions?

__
Johannes


Calvin Spealman

unread,
Oct 1, 2011, 1:21:08 AM10/1/11
to django-d...@googlegroups.com
On Fri, Sep 30, 2011 at 5:37 PM, Johannes Dollinger
<emul...@googlemail.com> wrote:
> The aim of this proposal is to reuse Q objects for models that are related through FK or M2M fields.
> A simplified example would be a Q object like
>
>    >>> is_blue = Q(blue=True)
>    >>> Thing.objects.filter(is_blue)
>
> that should be reused to filter the owners of things:
>
>    >>> User.objects.filter(things__blue=True).
>
> Currently, the only way to reuse the Q object would be subquery:
>
>    >>> User.objects.filter(things__in=Thing.objects.filter(is_blue)).
>
> I therefore propose the following API:
>
>    >>> Q(lookup=value).push('relfield')

I don't like this.

> which would be equivalent to
>
>    >>> Q(relfield__lookup=value)
>
> and
>
>    >>> qs.filter(relfield=Q(lookup=value))

I do like this

> which would be equivalent to
>
>    >>> qs.filter(Q(lookup=value).push('relfield')).

Again, not this.

Are you only proposal the push method as an implementation detail? If
so, don't let it be part of the API proposal itself.

> We then could write
>
>    >>> User.objects.filter(things=is_blue)
>
> which is shorter and produces more natural SQL than the subquery solution.
>
> Thoughs? Suggestions?
>
> __
> Johannes
>
>

> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
>

--
Read my blog! I depend on your acceptance of my opinion! I am interesting!
http://techblog.ironfroggy.com/
Follow me if you're into that sort of thing: http://www.twitter.com/ironfroggy

Javier Guerra Giraldez

unread,
Oct 3, 2011, 2:01:31 PM10/3/11
to django-d...@googlegroups.com
On Fri, Sep 30, 2011 at 4:37 PM, Johannes Dollinger
<emul...@googlemail.com> wrote:
> The aim of this proposal is to reuse Q objects for models that are related through FK or M2M fields.

i really want to have this feature! so i went and did a quick
implementation and created ticket #16979[1]

[1]:https://code.djangoproject.com/ticket/16979

really untested, but a variation that doesn't modify core seems to
work well (but the API is far too ugly to share)

--
Javier

Johannes Dollinger

unread,
Oct 4, 2011, 5:35:29 AM10/4/11
to django-d...@googlegroups.com

Thanks for starting this!
Why did you chose a staticmethod instead of an instancemethod?
While I'm not sold on the method name (perhaps prefix() or shift() or even __rshift__() would be clearer), I believe the ability to modify Q objects outside of filter()-Expressions is important.
Just like &, |, and ~, adding a prefix is an operation on Q objects. And the API should provide a way to manipulate them fully outside of QuerySets.

As an afterthought: The cleanest solution would probably be an implementation in Q.__init__.
The .push() method would then be unnecessary, as you could write Q(foo=Q(...)) instead of Q(..).push('foo').
And .filter()/.exclude() would just work, as they simply pass their arguments through to Q.__init__.

__
Johannes

Javier Guerra Giraldez

unread,
Oct 4, 2011, 8:02:56 AM10/4/11
to django-d...@googlegroups.com
On Tue, Oct 4, 2011 at 4:35 AM, Johannes Dollinger
<emul...@googlemail.com> wrote:
> Thanks for starting this!
> Why did you chose a staticmethod instead of an instancemethod?

because it's applied recursively not just to tree.Node, but to the
children and all members. it would be cleaner as a free function and
an instance method on tree.Node that just calls it.

> While I'm not sold on the method name (perhaps prefix() or shift() or even __rshift__() would be clearer), I believe the ability to modify Q objects outside of filter()-Expressions is important.

__rshift__() is too C++ for my taste :-) but I see the motivation.

> Just like &, |, and ~, adding a prefix is an operation on Q objects. And the API should provide a way to manipulate them fully outside of QuerySets.
>
> As an afterthought: The cleanest solution would probably be an implementation in Q.__init__.
> The .push() method would then be unnecessary, as you could write Q(foo=Q(...)) instead of Q(..).push('foo').
> And .filter()/.exclude() would just work, as they simply pass their arguments through to Q.__init__.

yes, that sounds cleaner. i'm still not sure if that should be on Q
or on tree.Node.

--
Javier

Javier Guerra Giraldez

unread,
Oct 4, 2011, 10:36:38 AM10/4/11
to django-d...@googlegroups.com
On Tue, Oct 4, 2011 at 4:35 AM, Johannes Dollinger
<emul...@googlemail.com> wrote:
> as you could write Q(foo=Q(...)) instead of Q(..).push('foo')

I thought that would work magically, since the Q object would just
pass that to the filter(); but a quick test proved me wrong.

I've just refactored as a Node._prefix() method that calls a
freestanding _prefix(t,pfx), and moved the relfield=Q smarts to the Q
constructor.

--
Javier

Reply all
Reply to author
Forward
0 new messages