Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#11789 closed Bug (fixed)

Aggregates ignore none()

Reported by: alexr 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:





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

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by russellm

  • milestone set to 1.2
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 6 years ago by noah

A patch for this ticket

comment:2 Changed 6 years ago by noah

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

comment:3 Changed 6 years ago by Alex

  • Triage Stage changed from Accepted to Ready for checkin

comment:4 Changed 6 years ago by jacob

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to 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 Changed 6 years ago by russellm

  • milestone changed from 1.2 to 1.3
  • Triage Stage changed from Accepted to 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 Changed 6 years ago by russellm

  • Component changed from Database layer (models, ORM) to ORM aggregation

comment:7 Changed 5 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to 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 Changed 5 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:9 Changed 5 years ago by ramiro

  • Resolution set to fixed
  • Status changed from assigned to 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 Changed 5 years ago by ramiro

  • 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 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:12 Changed 3 years ago by akaariai

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