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)