[Django] #24767: Add GREATEST and LEAST Query Expressions
Django
----------------------------------------------+--------------------
Reporter: Ian-Foote | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Postgres supports [http://www.postgresql.org/docs/current/static
/functions-conditional.html#FUNCTIONS-GREATEST-LEAST GREATEST and LEAST]
query expressions. These could easily be implemented in django by
subclassing the
[https://github.com/django/django/blob/master/django/db/models/functions.py#L8-L29
Coalesce] query expression and changing the `function` attribute to
`'GREATEST'` or `'LEAST'` respectively.
I'm not sure which other databases support this, so perhaps this can only
be added to `contrib.postgres`.
If this isn't appropriate for core, I'm happy to create a small PyPI
package instead.
--
Ticket URL: <https://code.djangoproject.com/ticket/24767>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Ian-Foote | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jarshwah):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Comment:
This is appropriate for core.
Oracle supports GREATEST and LEAST, any null results in null.
Sqlite has MAX(a, b, ..) and MIN(a, b, ..), any null results in null.
Mysql has GREATEST and LEAST, any null results in null if version >=
5.0.13 (django only supports >= 5.5 so no need to support both
behaviours).
I'm not certain how we should handle nulls. Should we allow backend
specific behaviour (probably not, historically) or try to decide which way
is "better" for Django.
As a rough idea we could require a "if_null=" kwarg that would produce
GREATEST( [COALESCE(arg, if_null) for args..] ).That'd produce the same
value on all backends, I think, at the cost of extraneous sql. There
should be an opt-out to get backend specific behaviour though.
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:1>
Django
Reporter: Ian-Foote | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ian-Foote):
For what it's worth, in my (current) use case the postgres handling of
`null` is ideal. I don't know how typical it is though.
My use case is annotating a model with a `last_updated` datetime. This is
calculated from the `created` datetime of two related models:
{{{
queryset.annotate(
last_update=Greatest(
Max('comments__created'),
Max('documents__created'),
),
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:2>
Django
Reporter: Ian-Foote | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jarshwah):
The postgres behaviour is the more useful IMO but it's also the odd one
out. But that's why I suggested wrapping everything in a coalesce while
providing opt outs.
I don't think we'd be able to emulate other backend behaviour with
postgres anyway without quite complex case expressions.
So as far as I'm concerned, if you can provide an expression that works
for postgres and emulates postgres on other backends (with an opt out),
it'd be accepted into core. Happy to help if needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:3>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ian-Foote):
* owner: nobody => Ian-Foote
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:4>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ian-Foote):
I've started work on this on
[https://github.com/django/django/compare/master...Ian-Foote:greatest-
least-expressions a branch]. It works for postgres, but I'm not sure how
to go about extending it to other databases. I assume I need to start
overwriting the `as_vendorname` methods, but it's not clear to me what to
put in them.
Also, should I be using `as_postgres` instead of defaulting to the
postgres behaviour?
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:5>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ian-Foote):
Ok, I've realised that overriding the `function` used for sqlite is
simple, so now I just need to add the emulation of postgres' null
handling.
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:6>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jarshwah):
If you open up a pull request now it'll make it easier for me (and others)
to provide some feedback inline with the code, even though it's not ready
yet.
Tip, check the coalesce() method of the ConcatPair. You'll need to figure
out the "default" value (by asking the user for it..) if an argument is
none though.
Also, sqlite requires at least 2 arguments. If there's only 1 argument
then it will use the aggregate MAX rather than the function mimicking
GREATEST. You could silently add a None expression to the list, require a
minimum of 2 expressions, or error out. But it should be handled some how.
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:7>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ian-Foote):
[https://github.com/django/django/pull/4634 Pull requested].
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:8>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ian-Foote):
Does it matter if sqlite uses the wrong `MAX` for one expression? I'm
having trouble creating a failing test for that.
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:9>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ian-Foote):
It looks like mysql also doesn't like one expression. Maybe requiring two
or more is the best solution for mysql and sqlite.
It also looks like the `coalesce` method doesn't help on mysql.
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:10>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Marc Tamlyn <marc.tamlyn@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"4ab53a558ac01c2dd7dafb8350cd72c630372335" 4ab53a55]:
{{{
#!CommitTicketReference repository=""
revision="4ab53a558ac01c2dd7dafb8350cd72c630372335"
Fixed #24767 -- Added Greatest and Least expressions
Greatest and Least are row-level Function versions of Min and Max.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:11>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6b418348558e7efc5bac459989887138a3045886" 6b41834]:
{{{
#!CommitTicketReference repository=""
revision="6b418348558e7efc5bac459989887138a3045886"
Minor edits to Greatest/Least docs; refs #24767.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:12>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* resolution: fixed =>
Comment:
The new tests are not passing on older versions of MySQL which don't
support microseconds (see [http://djangoci.com/job/django-master/
Jenkins]).
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:13>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"167a3203b645341a3b00b878594bfd8d4dd6b040" 167a3203]:
{{{
#!CommitTicketReference repository=""
revision="167a3203b645341a3b00b878594bfd8d4dd6b040"
Fixed tests for refs #24767 on databases that don't support microseconds.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:14>
Django
Reporter: Ian-Foote | Owner: Ian-Foote
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ian-Foote):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/24767#comment:15>