Opened 3 years ago

Last modified 4 weeks ago

#22550 assigned Bug

QuerySet last() and reverse() confusing when used with ordered slices

Reported by: Jon Dufresne Owned by: Michael Käufl
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


I ran all tests below with MySQL backend.

Given the following model:

class MyModel(models.Model):
    name = models.CharField(max_length=100)

    class Meta:
        ordering = ['name']

    def __unicode__(self):

From the following test script I receive the output:

import os
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "djtest.settings")
from myapp.models import MyModel

for i in range(10):
    m = MyModel(name='model%d' % i)

print MyModel.objects.all()[0:5]
print MyModel.objects.all()[0:5].last()
print MyModel.objects.all()[0:5].reverse()
[<MyModel: model0>, <MyModel: model1>, <MyModel: model2>, <MyModel: model3>, <MyModel: model4>]
[<MyModel: model9>, <MyModel: model8>, <MyModel: model7>, <MyModel: model6>, <MyModel: model5>, <MyModel: model4>, <MyModel: model3>, <MyModel: model2>, <MyModel: model1>, <MyModel: model0>]

In my opinion, the lines that show the output of last() and reverse() are confusing. My naive expectation was that each of these would be operating on the slice. That is, the last value from the slice or the reverse order of the slice.

[<MyModel: model4>, <MyModel: model3>, ...

Of course, after thinking about how this might get translated to SQL I understand why this happens. Making this a leaky abstraction. In my opinion, if expected behavior can't be realized an exception preventing the operation makes more sense.

The original reason I bumped into this is I was creating a custom Paginator to create page labels based on the first and last item on the page. As Paginator uses slices, calling last() produced unexpected (to me) results.

Change History (9)

comment:1 Changed 3 years ago by Matthias Erll

Triage Stage: UnreviewedAccepted

When retrying this using SQLite in the current master, additional sorting on the sliced queryset returns the same order. last() returns the first element from the default order, just like first().

Anyhow, as the queryset is already sliced, it can no longer be filtered or sorted. Explicit sorting with order_by() or using reverse()/last() on a sliced, unordered queryset raises an error message pointing this out. For consistency, the latter should also be implemented for a sliced queryset with a default sort order.

comment:2 Changed 3 years ago by Matthias Erll

Owner: changed from nobody to Matthias Erll
Status: newassigned

comment:3 Changed 3 years ago by Matthias Erll

Has patch: set

A PR has been sent, which fixes the behavior as described in ticket 22550. It also includes an addition to the docs, describing that sorting and filtering are not possible after the slicing operation:

Note: It is potentially breaking backwards compatibility in QuerySet.last() and QuerySet.reverse(). Using reverse() and last() on querysets was programmatically possible even after they had been sliced, provided that

  • models have defined ordering,
  • no further filters or sorting was used.

Although it has never delivered correct nor consistent results, these two functions may have been used under aforementioned conditions. After applying this patch, this will raise an error. In this process, the test for ticket 7235 had to be revised, as it was relying on the wrong order of operations.

comment:4 Changed 3 years ago by Tim Graham

Patch needs improvement: set

I left comments for improvement on PR. Please uncheck "Patch needs improvement" when you update it, thanks.

comment:5 Changed 2 months ago by Michael Käufl

comment:6 Changed 2 months ago by Michael Käufl

Owner: changed from Matthias Erll to Michael Käufl
Patch needs improvement: unset
Version: 1.6master

comment:7 Changed 2 months ago by Tim Graham

Patch needs improvement: set

comment:8 Changed 2 months ago by Michael Käufl

Patch needs improvement: unset

comment:9 Changed 4 weeks ago by Claude Paroz

#27997 was closed as a duplicate.

Note: See TracTickets for help on using tickets.
Back to Top