#33772: Queryset first() returns different result than queryset[0] when using
ExpressionWrapper
-------------------------------------+-------------------------------------
Reporter: Pablo Pissi | Owner: nobody
Type: Uncategorized | Status: closed
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
ExpressionWrapper,first,queryset | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
The addition of `order_by` columns to the `GROUP BY` clause specified by
`values().annotate()`
[https://docs.djangoproject.com/en/4.0/topics/db/aggregation/#interaction-
with-order-by is documented].
The crux of the issue remain; you are asking for an element at a
particular index of a collection with an undefined ordering. Adding
`order_by("example_model")` to your test case addresses the irregularity
you are experiencing with `first()`.
`first()` could arguably be adjusted to raise an exception when attempted
to be used on an unordered collection to ''refuse the temptation to
guess'' but that would require a deprecation period. Another approach
could be to only raise an exception when `first()` is called on a
unordered queryset with an explicit `values()` that doesn't include `pk`
which is the case reported here.
Here's what the code could look like for a systematic error on attempts to
use `first()` on a undefined ordering queryset performing aggregation over
specific columns
{{{#!diff
diff --git a/django/db/models/query.py b/django/db/models/query.py
index 67fcb411eb..da3d1ff3a7 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -1032,7 +1032,14 @@ async def alatest(self, *fields):
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]:
+ queryset = self
+ if not self.ordered:
+ if isinstance(self.query.group_by, tuple):
+ raise TypeError(
+ "Cannot use first() on an unordered queryset
performing aggregation"
+ )
+ queryset = self.order_by("pk")
+ for obj in queryset[:1]:
return obj
async def afirst(self):
}}}
And a somewhat more complex implementation that still allow the `pk`
{{{#!diff
diff --git a/django/db/models/query.py b/django/db/models/query.py
index 67fcb411eb..0534a2cc16 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -1032,7 +1032,17 @@ async def alatest(self, *fields):
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]:
+ queryset = self
+ if not self.ordered:
+ 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(
+ "Cannot use first() on an unordered queryset
performing aggregation "
+ "over a set of expressions that doesn't include the
base model's primary key"
+ )
+ queryset = self.order_by("pk")
+ for obj in queryset[:1]:
return obj
async def afirst(self):
}}}
The latter patch seems to be passing the test suite.
Thoughts on the value of such adjustment Carlton?
--
Ticket URL: <https://code.djangoproject.com/ticket/33772#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018146af4e5a-d9aae070-472e-4cf0-8ca1-84e27d249bb0-000000%40eu-central-1.amazonses.com.
