Provide option to chain QuerySet.order_by()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Provide option to chain QuerySet.order_by()

Markus Holtermann
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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

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

Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Adam Johnson-2
In reply to this post by Markus Holtermann
Seems like a good idea to me, I've been in a similar situation before and had to work around this part of QuerySets.

On 11 December 2017 at 21:10, Markus Holtermann <[hidden email]> wrote:
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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM2zxbTNP1fMnqEWZ0tJP54%3D_eccFON-VYvKVrktUPG1rg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Markus Holtermann
In reply to this post by Shai Berger
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

On 12/11/2017 10:27 PM, Shai Berger wrote:

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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/d1258e72-93a0-4075-da65-9f09902845f1%40markusholtermann.eu.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Adam Johnson-2
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.

On 11 December 2017 at 23:52, Markus Holtermann <[hidden email]> wrote:
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

On 12/11/2017 10:27 PM, Shai Berger wrote:
> 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.
>

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/d1258e72-93a0-4075-da65-9f09902845f1%40markusholtermann.eu.
For more options, visit 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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM3GHaVeKoxM6AgtH_o22Oz8s%3Dq53QVyRnZSitm96MoeMg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Shai Berger
On Fri, 15 Dec 2017 21:01:39 +0000
Adam Johnson <[hidden email]> 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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/20171218233351.5e08ebc8.shai%40platonix.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Mithlesh Kumar
In reply to this post by Markus Holtermann
Can you please tell me where the issues of django are in GitHub ? 

Thanks
Mithlesh K 

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/a976af9f-7f3a-4999-8379-54a9c3af3eb4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Adam Johnson-2
In reply to this post by Shai Berger
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)

On 18 December 2017 at 21:33, Shai Berger <[hidden email]> wrote:
On Fri, 15 Dec 2017 21:01:39 +0000
Adam Johnson <[hidden email]> 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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/20171218233351.5e08ebc8.shai%40platonix.com.
For more options, visit 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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM1G14n8fSuyAmuYBaevKNw%3DnWv7kkP6e-xj6y18-RUdfQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Adam Johnson-2
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.

On 19 December 2017 at 17:58, Adam Johnson <[hidden email]> 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)

On 18 December 2017 at 21:33, Shai Berger <[hidden email]> wrote:
On Fri, 15 Dec 2017 21:01:39 +0000
Adam Johnson <[hidden email]> 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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/20171218233351.5e08ebc8.shai%40platonix.com.
For more options, visit https://groups.google.com/d/optout.



--
Adam



--
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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM0SX20CO4PehFdQp5h%2Bz%2Bx2YwRBiB%3DNaKBgsAOne7Vvzg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Mithlesh Kumar
In reply to this post by Markus Holtermann
Thanks, Adam!

On Tuesday, December 12, 2017 at 2:40:11 AM UTC+5:30, Markus Holtermann wrote:
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 <a href="https://code.djangoproject.com/ticket/9415" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F9415\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGPV7iyjjqcOfHFu6zOa9yBlkNaKw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F9415\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGPV7iyjjqcOfHFu6zOa9yBlkNaKw&#39;;return true;">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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/0d96cf35-ff76-4147-b665-fa4b79024ad3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Shai Berger
In reply to this post by Adam Johnson-2
On Tue, 19 Dec 2017 17:58:35 +0000
Adam Johnson <[hidden email]> 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.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/20171223194238.1999df87.shai%40platonix.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Provide option to chain QuerySet.order_by()

Adam Johnson-2
Now you've said it, having both seems reasonable to me too :)

On 23 December 2017 at 17:42, Shai Berger <[hidden email]> wrote:
On Tue, 19 Dec 2017 17:58:35 +0000
Adam Johnson <[hidden email]> 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.

--
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 [hidden email].
To post to this group, send email to [hidden email].
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/20171223194238.1999df87.shai%40platonix.com.
For more options, visit 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 [hidden email].
To post to this group, send email to [hidden email].
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/CAMyDDM0%2Bf1CeBkLGwTqHXKicp5Q8xQodD8U3XzZLAwHvicXhvg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.