[Django] #28395: first() adds id field to group by clause on aggregation queries
Django
-------------------------------------+-------------------------------------
Reporter: John | Owner: nobody
Gresty |
Type: Bug | Status: new
Component: Database | Version: 1.11
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
When evaluating an aggregation query, for example:
{{{
MyModel.objects.values('group').annotate(value=Sum('value'))
}}}
works as expected. However adding {{{first()}}} to the end will add the id
field into the resulting queryset and as such the annotated value will be
the value of the first row, instead of the sum of the group.
{{{
>>> MyModel.objects.create(value=10)
<MyModel: MyModel object>
>>> MyModel.objects.create(value=20)
<MyModel: MyModel object>
>>> MyModel.objects.values('group').annotate(value=Sum('value'))
<QuerySet [{'group': 0, 'value': 30}]>
>>> MyModel.objects.values('group').annotate(value=Sum('value')).first()
{'group': 0, 'value': 10}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28395>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: John Gresty | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* component: Database layer (models, ORM) => Documentation
* version: 1.11 => master
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
The `first()` method requires an ordering to be specified else it would
return non-deterministic results hence why it's using `order_by('pk')`
when the queryset isn't ordered.
This is a bit similar to #10574 except the implicit ordering is added by
`first()` and not `_meta.ordering` so I'd suggest
[https://docs.djangoproject.com/en/1.11/topics/db/aggregation
/#interaction-with-default-ordering-or-order-by we link to the aggregation
section mentioning interaction with default ordering] or adjust the
documention to account for that.
IMHO it would have been better for `first()` to raise an exception when
called on an unordered queryset instead of implicitly choosing `pk` but in
your case I assume you want to either `order_by('group')` or `'value'`
before calling `first()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28395#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Botond
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* cc: Béres Botond (added)
* owner: nobody => Béres Botond
--
Ticket URL: <https://code.djangoproject.com/ticket/28395#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Béres
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
I've added a small patch that links from the first() definition to the
[https://docs.djangoproject.com/en/1.11/topics/db/aggregation
/#interaction-with-default-ordering-or-order-by aggregation section which
mentions interaction with default ordering]
--
Ticket URL: <https://code.djangoproject.com/ticket/28395#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Béres
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"95a14cfc47de5762ddb1400e6e5152f9e3172657" 95a14cf]:
{{{
#!CommitTicketReference repository=""
revision="95a14cfc47de5762ddb1400e6e5152f9e3172657"
Fixed #28395 -- Doc'd that QuerySet.first() can affect aggregation
queries.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28395#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Béres
Cleanup/optimization | Status: closed
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"854aec4801d74fa00a6695b58c95257dadc1fc83" 854aec48]:
{{{
#!CommitTicketReference repository=""
revision="854aec4801d74fa00a6695b58c95257dadc1fc83"
[2.0.x] Fixed #28395 -- Doc'd that QuerySet.first() can affect aggregation
queries.
Backport of 95a14cfc47de5762ddb1400e6e5152f9e3172657 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28395#comment:5>