Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#11789 closed Bug (fixed)

Aggregates ignore none()

Reported by: Alex Robbins Owned by: noah
Component: Database layer (models, ORM) Version: 1.1
Severity: Normal Keywords: aggregates
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX:

Description

Model.objects.all().aggregate(Max('field'))

and

Model.objects.none().aggregate(Max('field'))

seem to return the same thing. Aggregates pay attention to filters, but they seem to ignore the none() method. This isn't a common use, so it might not be worth fixing, except for correctness.

Attachments (1)

11789.diff (1.1 KB) - added by noah 7 years ago.
A patch for this ticket

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Triage Stage: UnreviewedAccepted

Changed 7 years ago by noah

Attachment: 11789.diff added

A patch for this ticket

comment:2 Changed 7 years ago by noah

Has patch: set
Owner: changed from nobody to noah
Status: newassigned

comment:3 Changed 7 years ago by Alex Gaynor

Triage Stage: AcceptedReady for checkin

comment:4 Changed 7 years ago by Jacob

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

I'm not sure that we want aggregates to return None; I think aggregates on an empty qs should raise an exception: "in the face of ambiguity, refuses the temptation to guess." Unless anyone's got a good reason to do it like this, let's make aggregate raise an exception on the empty set.

comment:5 Changed 7 years ago by Russell Keith-Magee

milestone: 1.21.3
Triage Stage: AcceptedDesign decision needed

I'm not sure I agree that this should raise an exception. Author.objects.none() is no different to Author.objects.filter(name="something that does not exist"); I don't see why the behaviour should be any different.

Either way, not critical for 1.2

comment:6 Changed 7 years ago by Russell Keith-Magee

Component: Database layer (models, ORM)ORM aggregation

comment:7 Changed 6 years ago by Malcolm Tredinnick

Triage Stage: Design decision neededAccepted

Russell is questioning the implementation, not the functionality. (I agree with no exception -- it's not an error). Moving back to "accepted"

comment:8 Changed 6 years ago by Julien Phalip

Severity: Normal
Type: Bug

comment:9 Changed 6 years ago by Ramiro Morales

Resolution: fixed
Status: assignedclosed

In [16254]:

Fixed #11789 -- Fixed aggregates so it interacts with QuerySet none() in a way consistent with other empty query sets. Thanks alexr for the report and patch.

comment:10 Changed 6 years ago by Ramiro Morales

Easy pickings: unset

Sorry for the error when giving credits in the commit message. Thanks goes to noah for providing the patch fixing this issue.

comment:11 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

comment:12 Changed 4 years ago by Anssi Kääriäinen

Component: ORM aggregationDatabase layer (models, ORM)
Note: See TracTickets for help on using tickets.
Back to Top