From ba9a2b754452b542d3f472f0acce6f940911aced Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 10 Mar 2021 09:16:28 +0100 Subject: [PATCH] Refs #32508 -- Raised TypeError instead of using "assert" on unsupported operations for sliced querysets. --- django/db/models/query.py | 42 +++++++++++++-------------- django/db/models/sql/query.py | 4 +-- docs/releases/4.0.txt | 3 ++ tests/delete/tests.py | 4 +++ tests/distinct_on_fields/tests.py | 5 ++++ tests/get_earliest_or_latest/tests.py | 10 +++++++ tests/lookup/tests.py | 5 ++++ tests/queries/tests.py | 15 ++++++---- tests/update/tests.py | 2 +- 9 files changed, 60 insertions(+), 30 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index ca72b23aea..387deca527 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -654,9 +654,6 @@ class QuerySet: "earliest() and latest() require either fields as positional " "arguments or 'get_latest_by' in the model's Meta." ) - - assert not self.query.is_sliced, \ - "Cannot change a query once a slice has been taken." obj = self._chain() obj.query.set_limits(high=1) obj.query.clear_ordering(force_empty=True) @@ -664,9 +661,13 @@ class QuerySet: return obj.get() def earliest(self, *fields): + if self.query.is_sliced: + raise TypeError('Cannot change a query once a slice has been taken.') return self._earliest(*fields) def latest(self, *fields): + if self.query.is_sliced: + raise TypeError('Cannot change a query once a slice has been taken.') return self.reverse()._earliest(*fields) def first(self): @@ -684,8 +685,8 @@ class QuerySet: Return a dictionary mapping each of the given IDs to the object with that ID. If `id_list` isn't provided, evaluate the entire QuerySet. """ - assert not self.query.is_sliced, \ - "Cannot use 'limit' or 'offset' with in_bulk" + if self.query.is_sliced: + raise TypeError("Cannot use 'limit' or 'offset' with in_bulk().") opts = self.model._meta unique_fields = [ constraint.fields[0] @@ -721,9 +722,8 @@ class QuerySet: def delete(self): """Delete the records in the current QuerySet.""" self._not_support_combined_queries('delete') - assert not self.query.is_sliced, \ - "Cannot use 'limit' or 'offset' with delete." - + if self.query.is_sliced: + raise TypeError("Cannot use 'limit' or 'offset' with delete().") if self.query.distinct or self.query.distinct_fields: raise TypeError('Cannot call delete() after .distinct().') if self._fields is not None: @@ -772,8 +772,8 @@ class QuerySet: fields to the appropriate values. """ self._not_support_combined_queries('update') - assert not self.query.is_sliced, \ - "Cannot update a query once a slice has been taken." + if self.query.is_sliced: + raise TypeError('Cannot update a query once a slice has been taken.') self._for_write = True query = self.query.chain(sql.UpdateQuery) query.add_update_values(kwargs) @@ -792,8 +792,8 @@ class QuerySet: code (it requires too much poking around at model internals to be useful at that level). """ - assert not self.query.is_sliced, \ - "Cannot update a query once a slice has been taken." + if self.query.is_sliced: + raise TypeError('Cannot update a query once a slice has been taken.') query = self.query.chain(sql.UpdateQuery) query.add_update_fields(values) # Clear any annotations so that they won't be present in subqueries. @@ -970,10 +970,8 @@ class QuerySet: return self._filter_or_exclude(True, args, kwargs) def _filter_or_exclude(self, negate, args, kwargs): - if args or kwargs: - assert not self.query.is_sliced, \ - "Cannot filter a query once a slice has been taken." - + if (args or kwargs) and self.query.is_sliced: + raise TypeError('Cannot filter a query once a slice has been taken.') clone = self._chain() if self._defer_next_filter: self._defer_next_filter = False @@ -1163,8 +1161,8 @@ class QuerySet: def order_by(self, *field_names): """Return a new QuerySet instance with the ordering changed.""" - assert not self.query.is_sliced, \ - "Cannot reorder a query once a slice has been taken." + if self.query.is_sliced: + raise TypeError('Cannot reorder a query once a slice has been taken.') obj = self._chain() obj.query.clear_ordering(force_empty=False) obj.query.add_ordering(*field_names) @@ -1175,8 +1173,8 @@ class QuerySet: Return a new QuerySet instance that will select only distinct results. """ self._not_support_combined_queries('distinct') - assert not self.query.is_sliced, \ - "Cannot create distinct fields once a slice has been taken." + if self.query.is_sliced: + raise TypeError('Cannot create distinct fields once a slice has been taken.') obj = self._chain() obj.query.add_distinct_fields(*field_names) return obj @@ -1185,8 +1183,8 @@ class QuerySet: order_by=None, select_params=None): """Add extra SQL fragments to the query.""" self._not_support_combined_queries('extra') - assert not self.query.is_sliced, \ - "Cannot change a query once a slice has been taken" + if self.query.is_sliced: + raise TypeError('Cannot change a query once a slice has been taken.') clone = self._chain() clone.query.add_extra(select, select_params, where, params, tables, order_by) return clone diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 5ca3f38e15..6c728d17bf 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -574,8 +574,8 @@ class Query(BaseExpression): """ assert self.model == rhs.model, \ "Cannot combine queries on two different base models." - assert not self.is_sliced, \ - "Cannot combine queries once a slice has been taken." + if self.is_sliced: + raise TypeError('Cannot combine queries once a slice has been taken.') assert self.distinct == rhs.distinct, \ "Cannot combine a unique query with a non-unique query." assert self.distinct_fields == rhs.distinct_fields, \ diff --git a/docs/releases/4.0.txt b/docs/releases/4.0.txt index 27f19043a9..3f04005a3a 100644 --- a/docs/releases/4.0.txt +++ b/docs/releases/4.0.txt @@ -318,6 +318,9 @@ Miscellaneous rather than via the recommended ``AdminSite.urls`` property, or ``AdminSite.get_urls()`` method. +* Unsupported operations on a sliced queryset now raise ``TypeError`` instead + of ``AssertionError``. + .. _deprecated-features-4.0: Features deprecated in 4.0 diff --git a/tests/delete/tests.py b/tests/delete/tests.py index 3dce135dd5..bf6543d735 100644 --- a/tests/delete/tests.py +++ b/tests/delete/tests.py @@ -275,6 +275,10 @@ class OnDeleteTests(TestCase): class DeletionTests(TestCase): + def test_sliced_queryset(self): + msg = "Cannot use 'limit' or 'offset' with delete()." + with self.assertRaisesMessage(TypeError, msg): + M.objects.all()[0:5].delete() def test_m2m(self): m = M.objects.create() diff --git a/tests/distinct_on_fields/tests.py b/tests/distinct_on_fields/tests.py index 34d4509cec..f87a3affb3 100644 --- a/tests/distinct_on_fields/tests.py +++ b/tests/distinct_on_fields/tests.py @@ -98,6 +98,11 @@ class DistinctOnTests(TestCase): c2 = c1.distinct('pk') self.assertNotIn('OUTER JOIN', str(c2.query)) + def test_sliced_queryset(self): + msg = 'Cannot create distinct fields once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): + Staff.objects.all()[0:5].distinct('name') + def test_transform(self): new_name = self.t1.name.upper() self.assertNotEqual(self.t1.name, new_name) diff --git a/tests/get_earliest_or_latest/tests.py b/tests/get_earliest_or_latest/tests.py index 96ebe19f32..0234deb195 100644 --- a/tests/get_earliest_or_latest/tests.py +++ b/tests/get_earliest_or_latest/tests.py @@ -81,6 +81,11 @@ class EarliestOrLatestTests(TestCase): Article.objects.model._meta.get_latest_by = ('pub_date', 'expire_date') self.assertEqual(Article.objects.filter(pub_date=datetime(2005, 7, 28)).earliest(), a4) + def test_earliest_sliced_queryset(self): + msg = 'Cannot change a query once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): + Article.objects.all()[0:5].earliest() + def test_latest(self): # Because no Articles exist yet, latest() raises ArticleDoesNotExist. with self.assertRaises(Article.DoesNotExist): @@ -143,6 +148,11 @@ class EarliestOrLatestTests(TestCase): Article.objects.model._meta.get_latest_by = ('pub_date', 'expire_date') self.assertEqual(Article.objects.filter(pub_date=datetime(2005, 7, 27)).latest(), a3) + def test_latest_sliced_queryset(self): + msg = 'Cannot change a query once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): + Article.objects.all()[0:5].latest() + def test_latest_manual(self): # You can still use latest() with a model that doesn't have # "get_latest_by" set -- just pass in the field name manually. diff --git a/tests/lookup/tests.py b/tests/lookup/tests.py index 548f470523..28ddc13aa0 100644 --- a/tests/lookup/tests.py +++ b/tests/lookup/tests.py @@ -244,6 +244,11 @@ class LookupTests(TestCase): with self.assertRaisesMessage(ValueError, msg % field_name): Model.objects.in_bulk(field_name=field_name) + def test_in_bulk_sliced_queryset(self): + msg = "Cannot use 'limit' or 'offset' with in_bulk()." + with self.assertRaisesMessage(TypeError, msg): + Article.objects.all()[0:5].in_bulk([self.a1.id, self.a2.id]) + def test_values(self): # values() returns a list of dictionaries instead of object instances -- # and you can specify which fields you want to retrieve. diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 1cbe005fa8..b9b0a72686 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -700,7 +700,8 @@ class Queries1Tests(TestCase): ) self.assertQuerysetEqual(q.reverse(), []) q.query.low_mark = 1 - with self.assertRaisesMessage(AssertionError, 'Cannot change a query once a slice has been taken'): + msg = 'Cannot change a query once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): q.extra(select={'foo': "1"}) self.assertQuerysetEqual(q.defer('meal'), []) self.assertQuerysetEqual(q.only('meal'), []) @@ -2359,15 +2360,18 @@ class QuerySetSupportsPythonIdioms(TestCase): ) def test_slicing_cannot_filter_queryset_once_sliced(self): - with self.assertRaisesMessage(AssertionError, "Cannot filter a query once a slice has been taken."): + msg = 'Cannot filter a query once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): Article.objects.all()[0:5].filter(id=1) def test_slicing_cannot_reorder_queryset_once_sliced(self): - with self.assertRaisesMessage(AssertionError, "Cannot reorder a query once a slice has been taken."): + msg = 'Cannot reorder a query once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): Article.objects.all()[0:5].order_by('id') def test_slicing_cannot_combine_queries_once_sliced(self): - with self.assertRaisesMessage(AssertionError, "Cannot combine queries once a slice has been taken."): + msg = 'Cannot combine queries once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): Article.objects.all()[0:1] & Article.objects.all()[4:5] def test_slicing_negative_indexing_not_supported_for_single_element(self): @@ -2417,7 +2421,8 @@ class WeirdQuerysetSlicingTests(TestCase): self.assertQuerysetEqual(Article.objects.all()[0:0], []) self.assertQuerysetEqual(Article.objects.all()[0:0][:10], []) self.assertEqual(Article.objects.all()[:0].count(), 0) - with self.assertRaisesMessage(TypeError, 'Cannot reverse a query once a slice has been taken.'): + msg = 'Cannot change a query once a slice has been taken.' + with self.assertRaisesMessage(TypeError, msg): Article.objects.all()[:0].latest('created') def test_empty_resultset_sql(self): diff --git a/tests/update/tests.py b/tests/update/tests.py index 588db40de6..0ba6fe43c3 100644 --- a/tests/update/tests.py +++ b/tests/update/tests.py @@ -130,7 +130,7 @@ class AdvancedTests(TestCase): """ method = DataPoint.objects.all()[:2].update msg = 'Cannot update a query once a slice has been taken.' - with self.assertRaisesMessage(AssertionError, msg): + with self.assertRaisesMessage(TypeError, msg): method(another_value='another thing') def test_update_respects_to_field(self):