Provide option to chain QuerySet.order_by()

171 views
Skip to first unread message

Markus Holtermann

unread,
Dec 11, 2017, 4:10:11 PM12/11/17
to Django developers (Contributions to Django itself)
Hi all,

I'm in the situation where I'd like to join two .order_by() calls on a QuerySet without losing the ordering set by the first call.

This was formerly discussed in https://code.djangoproject.com/ticket/9415 . I agree that simply changing the current behavior is not going to fly due to its backwards incompatibility.

My proposed API is similarly to force_insert/force_update on Model.save():

class QuerySet:
    def
order_by(self, *field_names, append=False, prepend=False):
       
if append and prepend:
           
raise AssertionError('Can only append or prepend, not both')
       
assert self.query.can_filter(), \
           
"Cannot reorder a query once a slice has been taken."
        obj
= self._chain()
       
if not append and not prepend:
            obj
.query.clear_ordering(force_empty=False)
        obj.query.add_ordering(*field_names, prepend=prepend)
        return obj


class Query:
    def add_ordering(self, *ordering
, prepend=False):
        if append and prepend:
           
raise AssertionError('Can only append or prepend, not both')
        # ...
        if ordering:
            if prepend:
                self.ordering = ordering + self.ordering
            else:
                self.order_by += ordering
        # ...

I'm happy to open a ticket once I got some feedback.

Cheers,

Markus

Shai Berger

unread,
Dec 11, 2017, 4:27:35 PM12/11/17
to django-d...@googlegroups.com
Hi Markus and all,

My instinct is that it's better to just make sure it's easy to find the current
ordering on a queryset. I suspect the use-case for modifications is not super
common, and I'd rather not bother with specifying the exact behavior of
nonsense cases like order_by(None, append=True) and
order_by('?', prepend=True) -- which, as the framework, we'd be required to
do, but if you're modifying a list on your own, you're free to ignore.

My 2 cents,
Shai.

Adam Johnson

unread,
Dec 11, 2017, 4:32:26 PM12/11/17
to django-d...@googlegroups.com
Seems like a good idea to me, I've been in a similar situation before and had to work around this part of QuerySets.

--
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/c9279d82-6b81-42cb-8577-16004e623e3d%40googlegroups.com.

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



--
Adam

Markus Holtermann

unread,
Dec 11, 2017, 6:53:21 PM12/11/17
to django-d...@googlegroups.com
Thanks for the input, Shai. I'd like to keep the current behavior
around. So .order_by(None) would still reset the ordering as-is.

But I agree, if we'de exposing QuerySet.query.order_by through a
documented API that would work for me as well (in fact, I'm using that
right now to work around the issue).

Cheers,

Markus

Adam Johnson

unread,
Dec 15, 2017, 4:02:17 PM12/15/17
to django-d...@googlegroups.com
I would prefer the QuerySet method than documenting parts of Query - QuerySet shouldn't have to make guarantees that its interactions with Query won't change, plus making Query in some way a public API could hamper features. Plus the amount of code is small and contained.

Also I think worrying about 'nonsense' cases is a bit of a distraction, we already support some 'nonsense' cases like .order_by('?', '?') because the API is trying to be as generic as possible. And order_by(None, append=True) could be made into a simple ValueError as it does indeed not make sense to ask for that.

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

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



--
Adam

Shai Berger

unread,
Dec 18, 2017, 4:34:13 PM12/18/17
to django-d...@googlegroups.com
On Fri, 15 Dec 2017 21:01:39 +0000
Adam Johnson <m...@adamj.eu> wrote:

> I would prefer the QuerySet method than documenting parts of Query -
> QuerySet shouldn't have to make guarantees that its interactions with
> Query won't change, plus making Query in some way a public API could
> hamper features. Plus the amount of code is small and contained.
>

I agree, but I think that method should be get_ordering().

> Also I think worrying about 'nonsense' cases is a bit of a
> distraction, we already support some 'nonsense' cases
> like .order_by('?', '?')

I grant that that's not a super-strong argument. However, making the
ordering available is also more flexible -- it allows you to do things
like copying the ordering between querysets, or modifying the ordering
based on existing ordering (e.g. reverse the ordering), or other things
I can't come up with off the top of my head, and which the suggested
API doesn't enable.

Shai

Mithlesh Kumar

unread,
Dec 19, 2017, 12:51:48 PM12/19/17
to Django developers (Contributions to Django itself)
Can you please tell me where the issues of django are in GitHub ? 

Thanks
Mithlesh K 

Adam Johnson

unread,
Dec 19, 2017, 12:59:15 PM12/19/17
to django-d...@googlegroups.com
Fair. I agree it's more flexible, but it does make "the most common case" of extending the ordering pretty long-winded:

ordering = qs.get_ordering()
ordering.append('new_field')
qs = qs.order_by(*ordering)

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

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



--
Adam

Adam Johnson

unread,
Dec 19, 2017, 1:04:15 PM12/19/17
to django-d...@googlegroups.com
Mithlesh, that's off-topic for this thread. But Django doesn't have it's issues on GitHub, it has them on Trac at https://code.djangoproject.com/ . See https://docs.djangoproject.com/en/dev/internals/contributing/ for a comprehensive guide on contributing.
--
Adam

Mithlesh Kumar

unread,
Dec 19, 2017, 6:42:33 PM12/19/17
to Django developers (Contributions to Django itself)
Thanks, Adam!

Shai Berger

unread,
Dec 23, 2017, 12:43:04 PM12/23/17
to django-d...@googlegroups.com
On Tue, 19 Dec 2017 17:58:35 +0000
Adam Johnson <m...@adamj.eu> wrote:

> Fair. I agree it's more flexible, but it does make "the most common
> case" of extending the ordering pretty long-winded:
>
> ordering = qs.get_ordering()
> ordering.append('new_field')
> qs = qs.order_by(*ordering)
>

I don't object to having both. I'd feel a little odd if we had APIs to
modify the ordering in non-trivial ways, but no API to retrieve it.

Adam Johnson

unread,
Dec 23, 2017, 12:54:31 PM12/23/17
to django-d...@googlegroups.com
Now you've said it, having both seems reasonable to me too :)

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

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



--
Adam
Reply all
Reply to author
Forward
0 new messages