From eee34ef64c026e3274c6d1b4fa2baffbc956b954 Mon Sep 17 00:00:00 2001 From: Matthias Erll Date: Sat, 17 May 2014 14:59:57 +0200 Subject: [PATCH] Fixed #22550 -- Prohibited QuerySet.last()/reverse() after slicing. --- django/db/models/query.py | 2 ++ docs/releases/2.0.txt | 12 ++++++++++++ docs/topics/db/queries.txt | 3 +++ tests/ordering/tests.py | 8 ++++++++ tests/queries/tests.py | 2 +- 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index a20237a229..fdf720cb8a 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -944,6 +944,8 @@ class QuerySet: def reverse(self): """Reverse the ordering of the QuerySet.""" + if not self.query.can_filter(): + raise TypeError('Cannot reverse a query once a slice has been taken.') clone = self._clone() clone.query.standard_ordering = not clone.query.standard_ordering return clone diff --git a/docs/releases/2.0.txt b/docs/releases/2.0.txt index 39df2d891d..044123485d 100644 --- a/docs/releases/2.0.txt +++ b/docs/releases/2.0.txt @@ -314,6 +314,18 @@ If you wish to keep this restriction in the admin when editing users, set admin.site.unregister(User) admin.site.register(User, MyUserAdmin) +``QuerySet.reverse()`` and ``last()`` are prohibited after slicing +------------------------------------------------------------------ + +Calling ``QuerySet.reverse()`` or ``last()`` on a sliced queryset leads to +unexpected results due to the slice being applied after reordering. This is +now prohibited, e.g.:: + + >>> Model.objects.all()[:2].reverse() + Traceback (most recent call last): + ... + TypeError: Cannot reverse a query once a slice has been taken. + Miscellaneous ------------- diff --git a/docs/topics/db/queries.txt b/docs/topics/db/queries.txt index b95173773b..b6263a9f42 100644 --- a/docs/topics/db/queries.txt +++ b/docs/topics/db/queries.txt @@ -361,6 +361,9 @@ every *second* object of the first 10:: >>> Entry.objects.all()[:10:2] +Further filtering or ordering of a sliced queryset is prohibited due to the +ambiguous nature of how that might work. + To retrieve a *single* object rather than a list (e.g. ``SELECT foo FROM bar LIMIT 1``), use a simple index instead of a slice. For example, this returns the first ``Entry`` in the database, after diff --git a/tests/ordering/tests.py b/tests/ordering/tests.py index de49626cf4..ff749331b9 100644 --- a/tests/ordering/tests.py +++ b/tests/ordering/tests.py @@ -203,6 +203,14 @@ class OrderingTests(TestCase): attrgetter("headline") ) + def test_no_reordering_after_slicing(self): + msg = 'Cannot reverse a query once a slice has been taken.' + qs = Article.objects.all()[0:2] + with self.assertRaisesMessage(TypeError, msg): + qs.reverse() + with self.assertRaisesMessage(TypeError, msg): + qs.last() + def test_extra_ordering(self): """ Ordering can be based on fields included from an 'extra' clause diff --git a/tests/queries/tests.py b/tests/queries/tests.py index b69ea84345..b2d5ef5e95 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -731,10 +731,10 @@ class Queries1Tests(TestCase): q.extra(select={'foo': "1"}), [] ) + self.assertQuerysetEqual(q.reverse(), []) q.query.low_mark = 1 with self.assertRaisesMessage(AssertionError, 'Cannot change a query once a slice has been taken'): q.extra(select={'foo': "1"}) - self.assertQuerysetEqual(q.reverse(), []) self.assertQuerysetEqual(q.defer('meal'), []) self.assertQuerysetEqual(q.only('meal'), [])