Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#28395 closed Cleanup/optimization (fixed)

first() adds id field to group by clause on aggregation queries

Reported by: John Gresty Owned by: Botond Béres
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Botond Béres Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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}

Change History (5)

comment:1 by Simon Charette, 7 years ago

Component: Database layer (models, ORM)Documentation
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization
Version: 1.11master

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 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().

comment:2 by Botond Béres, 6 years ago

Cc: Botond Béres added
Owner: changed from nobody to Botond Béres
Status: newassigned

comment:3 by Botond Béres, 6 years ago

Has patch: set

I've added a small patch that links from the first() definition to the aggregation section which mentions interaction with default ordering

Last edited 6 years ago by Botond Béres (previous) (diff)

comment:4 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 95a14cf:

Fixed #28395 -- Doc'd that QuerySet.first() can affect aggregation queries.

comment:5 by Tim Graham <timograham@…>, 6 years ago

In 854aec48:

[2.0.x] Fixed #28395 -- Doc'd that QuerySet.first() can affect aggregation queries.

Backport of 95a14cfc47de5762ddb1400e6e5152f9e3172657 from master

Note: See TracTickets for help on using tickets.
Back to Top