[Django] #31251: Unable to annotate a query that has an OuterRef already
Django
-------------------------------------+-------------------------------------
Reporter: OJFord | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 3.0
layer (models, ORM) | Keywords: db,outerref,group
Severity: Normal | by,subquery,annotate
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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
--
Ticket URL: <https://code.djangoproject.com/ticket/31251>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: OJFord | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage:
by,subquery,annotate | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by OJFord):
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
`type`s, whatever they are, not `OuterRef`, and then only after the `Min`
annotation `filter`ing on the `OuterRef("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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
by,subquery,annotate |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):
* type: Bug => Cleanup/optimization
* version: 3.0 => master
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
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):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Savin
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: nobody => Yuri Savin
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: Yuri
Type: | Savin
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Oliver Ford):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: Yuri
Type: | Savin
Cleanup/optimization | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
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")
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Yuri Savin):
* owner: Yuri Savin => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Yuri Savin):
Replying to [comment:5 felixxm]:
> 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")
> }}}
I think you can close this ticket better than i'm
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:7>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: (none) => Rohit Jha
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: Rohit Jha
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Rohit Jha):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: Rohit Jha
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: Rohit Jha
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db,outerref,group | Triage Stage: Accepted
by,subquery,annotate |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Rohit Jha (added)
* needs_tests: 1 => 0
Comment:
Tests => https://github.com/django/django/pull/12489
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Oliver Ford | Owner: Rohit Jha
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Keywords: db,outerref,group | Triage Stage: Ready for
by,subquery,annotate | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"486786c4c480f5b1ae3ac1c95858b03699b9eb9c" 486786c]:
{{{
#!CommitTicketReference repository=""
revision="486786c4c480f5b1ae3ac1c95858b03699b9eb9c"
Fixed #31251 -- Disabled grouping by OuterRef() annotation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31251#comment:13>