summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPablo Pissi <pablopissi@gmail.com>2022-06-12 22:08:21 -0300
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2022-06-14 12:24:43 +0200
commitd287294885e116c49c87f0c663068511a8402c0f (patch)
tree19171842b0581e62dbf5eda6a56a66a37651c53a
parentdb588d4f0edde5095f8975be1da4e693268b86b3 (diff)
Fixed #33772 -- Added QuerySet.first()/last() error message on unordered queryset with aggregation.
-rw-r--r--django/db/models/query.py23
-rw-r--r--tests/get_earliest_or_latest/models.py5
-rw-r--r--tests/get_earliest_or_latest/tests.py21
-rw-r--r--tests/gis_tests/geoapp/test_expressions.py1
4 files changed, 47 insertions, 3 deletions
diff --git a/django/db/models/query.py b/django/db/models/query.py
index 9cfe118e79..308073d4de 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -1032,7 +1032,12 @@ class QuerySet:
def first(self):
"""Return the first object of a query or None if no match is found."""
- for obj in (self if self.ordered else self.order_by("pk"))[:1]:
+ if self.ordered:
+ queryset = self
+ else:
+ self._check_ordering_first_last_queryset_aggregation(method="first")
+ queryset = self.order_by("pk")
+ for obj in queryset[:1]:
return obj
async def afirst(self):
@@ -1040,7 +1045,12 @@ class QuerySet:
def last(self):
"""Return the last object of a query or None if no match is found."""
- for obj in (self.reverse() if self.ordered else self.order_by("-pk"))[:1]:
+ if self.ordered:
+ queryset = self.reverse()
+ else:
+ self._check_ordering_first_last_queryset_aggregation(method="last")
+ queryset = self.order_by("-pk")
+ for obj in queryset[:1]:
return obj
async def alast(self):
@@ -1926,6 +1936,15 @@ class QuerySet:
if self.query.combinator or other.query.combinator:
raise TypeError(f"Cannot use {operator_} operator with combined queryset.")
+ def _check_ordering_first_last_queryset_aggregation(self, method):
+ if isinstance(self.query.group_by, tuple) and not any(
+ col.output_field is self.model._meta.pk for col in self.query.group_by
+ ):
+ raise TypeError(
+ f"Cannot use QuerySet.{method}() on an unordered queryset performing "
+ f"aggregation. Add an ordering with order_by()."
+ )
+
class InstanceCheckMeta(type):
def __instancecheck__(self, instance):
diff --git a/tests/get_earliest_or_latest/models.py b/tests/get_earliest_or_latest/models.py
index 936011ab6b..bbf2075d36 100644
--- a/tests/get_earliest_or_latest/models.py
+++ b/tests/get_earliest_or_latest/models.py
@@ -16,6 +16,11 @@ class Person(models.Model):
# Note that this model doesn't have "get_latest_by" set.
+class Comment(models.Model):
+ article = models.ForeignKey(Article, on_delete=models.CASCADE)
+ likes_count = models.PositiveIntegerField()
+
+
# Ticket #23555 - model with an intentionally broken QuerySet.__iter__ method.
diff --git a/tests/get_earliest_or_latest/tests.py b/tests/get_earliest_or_latest/tests.py
index cd63adb75d..21692590cc 100644
--- a/tests/get_earliest_or_latest/tests.py
+++ b/tests/get_earliest_or_latest/tests.py
@@ -1,8 +1,9 @@
from datetime import datetime
+from django.db.models import Avg
from django.test import TestCase
-from .models import Article, IndexErrorArticle, Person
+from .models import Article, Comment, IndexErrorArticle, Person
class EarliestOrLatestTests(TestCase):
@@ -245,3 +246,21 @@ class TestFirstLast(TestCase):
expire_date=datetime(2005, 9, 1),
)
check()
+
+ def test_first_last_unordered_qs_aggregation_error(self):
+ a1 = Article.objects.create(
+ headline="Article 1",
+ pub_date=datetime(2005, 7, 26),
+ expire_date=datetime(2005, 9, 1),
+ )
+ Comment.objects.create(article=a1, likes_count=5)
+
+ qs = Comment.objects.values("article").annotate(avg_likes=Avg("likes_count"))
+ msg = (
+ "Cannot use QuerySet.%s() on an unordered queryset performing aggregation. "
+ "Add an ordering with order_by()."
+ )
+ with self.assertRaisesMessage(TypeError, msg % "first"):
+ qs.first()
+ with self.assertRaisesMessage(TypeError, msg % "last"):
+ qs.last()
diff --git a/tests/gis_tests/geoapp/test_expressions.py b/tests/gis_tests/geoapp/test_expressions.py
index d0e92a9599..b56832bb6f 100644
--- a/tests/gis_tests/geoapp/test_expressions.py
+++ b/tests/gis_tests/geoapp/test_expressions.py
@@ -62,6 +62,7 @@ class GeoExpressionsTests(TestCase):
)
qs = (
City.objects.values("name")
+ .order_by("name")
.annotate(
distance=Min(
functions.Distance("multifields__point", multi_field.city.point)