[Django] #24747: Allow transforms in order_by
Django
----------------------------------------------+--------------------
Reporter: sparkyb | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
The extension of lookups to support transforms is great, but I'd like to
be able to order by a transformed field.
As a use case, let's say I define a length transform. I filter by length
such as {{{Song.objects.filter(title__length__gt=10)}}}, but I think it
makes sense that I ought to also be able to sort by this transform, for
instance: {{{Song.objects.all().order_by('title__length')}}}
The related ticket #24629 points out that Func expressions should be
usable as transforms. Although in 1.8 I could get the desired sort by
ordering by the Length func expression, I feel that the reverse should
also be true, that I should be able to order by a transform.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: sparkyb | 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):
* cc: josh.smeaton@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* version: 1.8 => master
* needs_docs: => 0
* stage: Unreviewed => Accepted
Comment:
This is the general problem that matches these specific tickets:
https://code.djangoproject.com/ticket/24592
https://code.djangoproject.com/ticket/23709
And copying my comment from 24592:
There are possibly two ways to fix this situation.
1. Make all Transforms into Expressions. That is, we need to consolidate
the very close APIs of Transforms/Lookups and Expressions. We have and we
are discussing this elsewhere (though I can't find where just at the
moment). This is the most likely solution we'll converge on.
2. Make .values and .order_by understand the transformation/lookup API.
We'll probably steer clear of this as it helps to widen the gap between
transforms/expressions.
I now think we should implement both, although 2 will be dependent one 1
(where 1 is tracked here: https://code.djangoproject.com/ticket/24629).
The idea is that transforms should be expressions, and then converting the
transform syntax into the expression syntax internally would allow users
to use whichever syntax they'd prefer.
I'll close the other tickets that are asking for specific transforms and
leave this one open as it calls out the general issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:1>
Django
Reporter: sparkyb | 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 schinckel):
This also makes sense within the context of structured fields, like Array,
JSON and Hstore: you may want to order by a lookup within the field.
I believe this will be addressed by this proposed solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:2>
Django
Reporter: sparkyb | 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 artursmet):
Also distinct by json/hstore field's key is not possible now. And it looks
like, there is no workaround with Expressions because DISTINCT queries are
not expressions as such.
Are there any plans, when this feature will be supported?
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:3>
Django
Reporter: sparkyb | 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):
I had a quick look at this a few weeks back, and while I didn't actually
implement any of it, I think I found the place where the work would need
to be done.
https://github.com/jarshwah/django/commit/0522ccd1b4f7867f662de238ec9437dbed72f0d9
I would hope to get this done before django 1.10, but my time lately has
been totally consumed by day-job and family.
As far as distinct queries go, distinct would have to support F()
expressions internally for this patch to have any effect there. I'm not
even certain that this would be enough, because I think you'd still need
to have the field__transform in the select list. I haven't played with
this enough to know though.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:4>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
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 Matthew Wilkes):
* owner: nobody => Matthew Wilkes
* status: new => assigned
Comment:
I'm having a go at this between talks at PyCon.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:5>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
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 Tim Graham):
WIP [https://github.com/django/django/pull/8528 PR] from Matthew
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:6>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
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 Austin Roberts):
As I noted on #28168, I found a work around for this, though it would
still be great to have this work the same way as filter.
For now, I'm able to work around this by instead doing:
{{{
from django.db.models.expressions import OrderBy, RawSQL
MyModel.objects.all().order_by(OrderBy(RawSQL("metadata #> '{some}' #>
'{field}'", []))
}}}
Similarly, I have cases where I want to be able to do
{{{
MyModel.objects.all().values("metadata__some__field")
}}}
I've worked out a similar work around of
{{{
MyModel.objects.all().annotate(some_field=RawSQL("metadata #> '{some}' #>
'{field}'", [])).values("some_field")
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:7>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
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 Matthew Wilkes):
The RawSQL trick is a nice workaround. For my part, I've done some work on
this and have order_by, distinct and values working on JSON fields, as
well as order_by on a toy lookup for testing purposes.
The PR isn't in a mergable state yet, as I've left lots of debugging help
in for myself and not tidied the commits yet, but I'm open to ideas for
additional things I should test.
Austin, do the tests at https://github.com/django/django/pull/8528/files
#diff-7a482072a1e7be83b1c715879401a4f6 accurately represent what you want
from JSON fields?
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:8>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Matthew Wilkes):
* has_patch: 0 => 1
* stage: Accepted => Unreviewed
Comment:
I've updated this PR to have what I think is a good implementation of this
problem. I'd appreciate review, especially from Josh as he started working
on it.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:9>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:10>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Matthew Wilkes):
I know the depths of the ORM mean there's not many people who feel
comfortable reviewing changes, but if anyone does have time to take a look
and see if this needs more work I'd be very grateful. Happy to provide
beer-based bribes, if that'll help.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:11>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Josh Smeaton):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* needs_docs: 0 => 1
Comment:
A few things to tidy up or consider. Need some usage documentation. Need
some tests outside contrib/postgres showcasing new abilities. Old docs
might need to be changed to eliminate redundant calls like
annotate().values().annotate().
I don't see anything fundamentally wrong though - it's very nearly ready
for a merge I think. I'll know a lot more once I've actually used the
changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:12>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by Matthew Wilkes):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
I'm unsetting these markers as since Josh set them I've added more tests
and notes in the documentation, so I think it's ready to be looked at
again. I've also rebased against master and put the documentation changes
in against 2.1.
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:13>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
I added a couple of small points on the PR.
I have a query whether we want to emphasise the new functionality a bit
more in the docs (or release notes) but I'd accept a "No" on that quite
happily.
Beyond that, if nobody has any particular suggestions they want
incorporated, this looks good to go to me. (My thought would be to allow a
few days for feedback. If none is coming advance to Ready for Checkin.)
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:14>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:15>
Django
Reporter: Ben Buchwald | Owner: Matthew
| Wilkes
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"2162f0983de0dfe2178531638ce7ea56f54dd4e7" 2162f09]:
{{{
#!CommitTicketReference repository=""
revision="2162f0983de0dfe2178531638ce7ea56f54dd4e7"
Fixed #24747 -- Allowed transforms in QuerySet.order_by() and
distinct(*fields).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24747#comment:16>