Opened 7 years ago

Closed 5 years ago

Last modified 2 years ago

#8784 closed Uncategorized (wontfix)

Add HAVING-clauses to QuerySets

Reported by: Johntron Owned by: Johntron
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: db query having clause extra database
Cc: klemens@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We need some way to include HAVING clauses in QuerySets. The HAVING clause allows aliases to be used as operands in conditionals. The db.models.QuerySet.extra() looks like the best method to use for something like this, so I've added some code to db.models.QuerySet.extra() method and the corresponding db.models.sql.query.Query class. See patch.

A use case:

locations = Location.objects.extra( select={'distance': 'SQRT(POW(69.1 * (lattitude - %f), 2) + POW(69.1 * (longitude - %f) * COS(lattitude/57.3), 2))' % (lat, lng)} )
locations = locations.extra( having=['distance < %f' % maxdist] )

Attachments (2)

diff.txt (3.3 KB) - added by Johntron 7 years ago.
Patch to add HAVING clauses to QuerySets
patch.diff (3.3 KB) - added by Johntron 7 years ago.
Patch to add HAVING clauses to QuerySets

Download all attachments as: .zip

Change History (23)

comment:1 Changed 7 years ago by Johntron

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to Johntron
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 7 years ago by Johntron

  • Needs documentation set

comment:3 Changed 7 years ago by Johntron

  • Needs tests set

Changed 7 years ago by Johntron

Patch to add HAVING clauses to QuerySets

comment:4 Changed 7 years ago by russellm

  • Resolution set to duplicate
  • Status changed from assigned to closed

This is essentially a very small subset of the work contained in #3566. This ticket will be integrated soon after the 1.0 release.

Changed 7 years ago by Johntron

Patch to add HAVING clauses to QuerySets

comment:5 Changed 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:6 Changed 5 years ago by fetzig

I used to do this by hacking a
where="1 HAVING max_distance < 50".

in django trunk (head revision) this doesn't work anymore. and the patch of this ticket would need an update to work (i tried it and failed). don't want to go into detail ;)

how can i accomplish this (adding a filter which needs to be a having clause in mysql) now. this is a subset of #3566? but i cant figure out how to do this. who to accomplish the distance calculations by calling annotate()? seems that i misunderstand something, because i cant put it together.

thx for your time.

comment:7 Changed 5 years ago by fetzig

  • Cc klemens@… added

comment:8 Changed 5 years ago by fetzig

  • Resolution duplicate deleted
  • Status changed from closed to reopened

comment:9 Changed 5 years ago by kmtracey

  • Resolution set to duplicate
  • Status changed from reopened to closed

You should switch to using the implemented aggregation features for queries that require HAVING. If you are unsure how to build the query you are looking for using the Django aggregation support, #django IRC or the django-users list would be places to ask for help. If the existing aggregation features do not support what you need, a new ticket (if one does not already exist) might be appropriate. Such a ticket would describe the requirement in terms of the aggregate query function that is missing from Django's ORM, not in terms of adding ways to specify bits of SQL. Re-opening this ticket is not a productive way forward; Django's ORM is not going to move in the direction of providing more ways to specify bits of SQL via extra().

comment:10 Changed 5 years ago by fetzig

sorry but duplicate is just an invalid resolution for this ticket.

the aggregation feature is far from being able to calculate something like

distance='SQRT(POW(69.1 * (lattitude - %f), 2) + POW(69.1 * (longitude - %f) * COS(lattitude/57.3), 2))' % (lat, lng)

Django's ORM is not going to move in the direction of providing more ways to specify bits of SQL via extra().

i totally agree. but whats the point in having extra()-extra field if you get an exception if you try to user them in filter()!?

so maybe it's not valid to reopen this ticket. but i think its not valid to say its a duplicate of #3566. how is it possible to do whats described in the ticket description witch the aggregation features?

comment:11 Changed 5 years ago by kmtracey

  • Resolution duplicate deleted
  • Status changed from closed to reopened

comment:12 follow-up: Changed 5 years ago by kmtracey

  • Resolution set to wontfix
  • Status changed from reopened to closed

The ORM will not be growing a way to specify HAVING in extra(). Aggregation offers ways of retrieving the kinds of results you can get with HAVING. If the existing aggregation features are not sufficient for some needs, then requests for improvement in this area should focus on improvements to the aggregation features, not direct addition of HAVING in extra(). Alternatively, perhaps raw SQL might be an alternative solution.

comment:13 follow-up: Changed 5 years ago by kmtracey

And questions on how to accomplish a particular query for a given set of models are better asked in #django IRC or django-users. The ticket tracker simply is not a good place for these types of questions: they don't get the right audience.

comment:14 in reply to: ↑ 13 Changed 5 years ago by fetzig

Replying to kmtracey:

And questions on how to accomplish a particular query for a given set of models are better asked in #django IRC or django-users. The ticket tracker simply is not a good place for these types of questions: they don't get the right audience.

it was a rhetorical question to describe my opinion on this issue.

comment:15 in reply to: ↑ 12 Changed 5 years ago by fetzig

Replying to kmtracey:

The ORM will not be growing a way to specify HAVING in extra(). Aggregation offers ways of retrieving the kinds of results you can get with HAVING. If the existing aggregation features are not sufficient for some needs, then requests for improvement in this area should focus on improvements to the aggregation features, not direct addition of HAVING in extra().

requested, and there is an other ticket #11671 with describes the same/similar issue. but 8 months old...and it looks like nobody cares. but i'll make a new ticket and send you a message when its rejected sorry for that

Alternatively, perhaps raw SQL might be an alternative solution.

RawQuerySets can't be filtered. are not directly compatible with the Paginator()... but thx for the hint.

comment:16 Changed 5 years ago by kmtracey

If there is another ticket that describes what you are looking for then please try to contribute to getting that ticket moved forward rather than opening a new one. And please recognize that no activity does not imply "no one cares". Everyone who contributes to Django does it in spare time and there just is not enough spare time available to do everything everyone would like to do as quickly as one might hope.

(I wasn't actually suggesting raw query sets, I was suggesting plain raw sql.)

comment:17 Changed 3 years ago by ykaganovich@…

  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

What will it take to change the devs' minds on this ticket? This is clearly a huge deficiency for anyone using Django with MySQL backend. I can use extra/select to create custom annotations (on arbitrary subselects that aggregation can't do), I can use extra/order_by to order on these annotations, but I can't use extra/having to filter on these annotations... I run into this issue probably once a week. There's absolutely no justification for not supporting HAVING clause.

And no, Django builtin annotation/aggregation support is not anywhere adequate and never will be. And no, using raw sql or raw query sets are not good alternatives until there's a way to get back from those to plain old QuerySets that I can further refine using Django ORM magic.

More to the point, why is this an issue? It seems like a trivial enhancement that addresses a huge pain point.

comment:18 Changed 3 years ago by akaariai

While I would really, really like to get rid of the whole qs.extra() in the long term, it is likely going to stay around for a (long) while still. So, I am cautiously +0 on adding the having arg to extra.

A nice clean patch demonstrating that this is easy to implement and this actually works (that is, sufficient tests included), and then posting to django-developers about this issue is the way to go. I can't guarantee inclusion. And, if the patch is anything but "obviously correct, can't break anything", then I object to adding this. My gut feeling is that the patch is likely going to be simple.

comment:19 Changed 3 years ago by hcarvalhoalves

Ticket #19434 is related to this. The API doesn't accept aliases on methods (filter(), etc.), neither does it expose the HAVING clause. That means not only it makes it hard to use aliases, it even goes the extra mile by *keeping* you from writing HAVING clauses, and you are left high and dry in the ORM.

Resorting raw is not a solution, since it's not functionally equivalent to QuerySet objects. The whole argument about not having SQL specific features on the ORM is moot too, since the only thing the ORM supports right now is SQL.

I feel this should be reopened and discussed further. Dealing with aliases is not uncommon and the ORM lacks terribly on this field now.

Version 1, edited 3 years ago by hcarvalhoalves (previous) (next) (diff)

comment:20 Changed 2 years ago by anonymous

There is an extremely basic use here that fetzig alluded to earlier: calculating distances based on long/lat. It would be nice if Django's ORM supported a way to derive and filter on things like this.

comment:21 Changed 2 years ago by fetzig

Replying to anonymous:

There is an extremely basic use here that fetzig alluded to earlier: calculating distances based on long/lat. It would be nice if Django's ORM supported a way to derive and filter on things like this.

thx ;) but IMHO its way better to use geo-django. put a lot of effort into this without geo-django because i thought its to much of a hustle (postgis and all the other dependencies). but after i hit the wall (this ticket) i refactored my project within a couple of hours.

in addition to that i'm convinced this ticket was handled the right way. you're right "it would be nice" but it's simply wrong or inelegant. don't have the time/motivation to describe exactly why, but i guess a look at the other comments and a more detailed look into the ORM should convince you if you don't "believe" me...wasted a couple of days for being a disbeliever myself...at least i'm wiser now ;)

Last edited 2 years ago by fetzig (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top