Fixed #33772 -- Added QuerySet.first()/last() error message on unordered queryset with aggregation.

This commit is contained in:
Pablo Pissi 2022-06-12 22:08:21 -03:00 committed by Mariusz Felisiak
parent db588d4f0e
commit d287294885
4 changed files with 47 additions and 3 deletions

View File

@ -1032,7 +1032,12 @@ class QuerySet:
def first(self): def first(self):
"""Return the first object of a query or None if no match is found.""" """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 return obj
async def afirst(self): async def afirst(self):
@ -1040,7 +1045,12 @@ class QuerySet:
def last(self): def last(self):
"""Return the last object of a query or None if no match is found.""" """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 return obj
async def alast(self): async def alast(self):
@ -1926,6 +1936,15 @@ class QuerySet:
if self.query.combinator or other.query.combinator: if self.query.combinator or other.query.combinator:
raise TypeError(f"Cannot use {operator_} operator with combined queryset.") 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): class InstanceCheckMeta(type):
def __instancecheck__(self, instance): def __instancecheck__(self, instance):

View File

@ -16,6 +16,11 @@ class Person(models.Model):
# Note that this model doesn't have "get_latest_by" set. # 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. # Ticket #23555 - model with an intentionally broken QuerySet.__iter__ method.

View File

@ -1,8 +1,9 @@
from datetime import datetime from datetime import datetime
from django.db.models import Avg
from django.test import TestCase from django.test import TestCase
from .models import Article, IndexErrorArticle, Person from .models import Article, Comment, IndexErrorArticle, Person
class EarliestOrLatestTests(TestCase): class EarliestOrLatestTests(TestCase):
@ -245,3 +246,21 @@ class TestFirstLast(TestCase):
expire_date=datetime(2005, 9, 1), expire_date=datetime(2005, 9, 1),
) )
check() 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()

View File

@ -62,6 +62,7 @@ class GeoExpressionsTests(TestCase):
) )
qs = ( qs = (
City.objects.values("name") City.objects.values("name")
.order_by("name")
.annotate( .annotate(
distance=Min( distance=Min(
functions.Distance("multifields__point", multi_field.city.point) functions.Distance("multifields__point", multi_field.city.point)