Opened 15 years ago

Closed 13 years ago

Last modified 12 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: no

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 15 years ago.
A patch for this ticket

Download all attachments as: .zip

Change History (13)

comment:1 by Russell Keith-Magee, 15 years ago

milestone: → 1.2
Triage Stage: Unreviewed → Accepted

by noah, 15 years ago

Attachment: 11789.diff​ added

A patch for this ticket

comment:2 by noah, 15 years ago

Has patch: set
Owner: changed from nobody to noah
Status: new → assigned

comment:3 by Alex Gaynor, 15 years ago

Triage Stage: Accepted → Ready for checkin

comment:4 by Jacob, 15 years ago

Patch needs improvement: set
Triage Stage: Ready for checkin → Accepted

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 by Russell Keith-Magee, 15 years ago

milestone: 1.2 → 1.3
Triage Stage: Accepted → Design 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 by Russell Keith-Magee, 15 years ago

Component: Database layer (models, ORM) → ORM aggregation

comment:7 by Malcolm Tredinnick, 14 years ago

Triage Stage: Design decision needed → Accepted

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

comment:8 by Julien Phalip, 13 years ago

Severity: → Normal
Type: → Bug

comment:9 by Ramiro Morales, 13 years ago

Resolution: → fixed
Status: assigned → closed

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 by Ramiro Morales, 13 years ago

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 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

comment:12 by Anssi Kääriäinen, 12 years ago

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