Opened 6 years ago
Closed 6 years ago
#31251 closed Cleanup/optimization (fixed)
QuerySet.annotate() crashes when grouping by OuterRef().
| Reported by: | Oliver Ford | Owned by: | Rohit Jha |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | db, outerref, group by, subquery, annotate |
| Cc: | Rohit Jha | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
I'm trying to construct a query like:
cheapest_query = models.Subquery(qs.annotate(
groupbytype=models.OuterRef("type"),
).values("groupbytype")).annotate(
cheapest=models.Min("price"),
).values("cheapest"))
qs = qs.annotate(premium_over_cheapest=models.F("price") - cheapest_query)
However this fails in the second annotate (cheapest=) since ResolvedOuterRef does not descend from BaseExpression; so it has no get_group_by_cols method:
File "/usr/local/lib/python3.8/site-packages/django/db/models/query.py", line 1078, in annotate
clone.query.set_group_by()
File "/usr/local/lib/python3.8/site-packages/django/db/models/sql/query.py", line 1938, in set_group_by
inspect.getcallargs(annotation.get_group_by_cols, alias=alias)
AttributeError: 'ResolvedOuterRef' object has no attribute 'get_group_by_cols'
https://github.com/django/django/blob/master/django/db/models/expressions.py
Change History (13)
comment:1 by , 6 years ago
comment:2 by , 6 years ago
| Easy pickings: | set |
|---|---|
| Summary: | Unable to annotate a query that has an OuterRef already → QuerySet.annotate() crashes when grouping by OuterRef(). |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → Cleanup/optimization |
| Version: | 3.0 → master |
Grouping by OuterRef() doesn't have much value, it's like using a constant value, so IMO we can easily fix this with:
diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py
index 2e831b21ac..a165923784 100644
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -562,6 +562,9 @@ class ResolvedOuterRef(F):
def relabeled_clone(self, relabels):
return self
+ def get_group_by_cols(self, alias=None):
+ return []
+
class OuterRef(F):
def resolve_expression(self, *args, **kwargs):
comment:3 by , 6 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:4 by , 6 years ago
Why not the column that OuterRef is referencing? That was my intended behaviour, though I eventually realised I was overcomplicating it and just needed to group on the column directly, not an outer ref, and filter on the outer ref.
follow-up: 7 comment:5 by , 6 years ago
Oliver, OuterRef() is a field from the outer query, so it will be the same for all rows in a subquery. Is there any value in adding it to a GROUP BY clause? e.g.
(SELECT MIN("price") from "subquery_table" GROUP BY "outer_table"."type")
is equivalent to
(SELECT MIN("price") from "subquery_table")
comment:6 by , 6 years ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
comment:8 by , 6 years ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:9 by , 6 years ago
| Has patch: | set |
|---|
comment:10 by , 6 years ago
| Needs tests: | set |
|---|---|
| Patch needs improvement: | set |
comment:11 by , 6 years ago
| Cc: | added |
|---|---|
| Needs tests: | unset |
comment:12 by , 6 years ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
Perhaps I was doing something silly in the first place; I've resolved my issue by changing the subquery to first annotate (group by) all the
types, whatever they are, notOuterRef, and then only after theMinannotationfiltering on theOuterRef("type")matching the grouped by type.I'll leave this open for now though in case it's desired to improve the error message in this case, or perhaps there's a more sensibly-intentioned query that does produce the same error.