Opened 9 years ago

Closed 6 years ago

#24747 closed New feature (fixed)

Allow transforms in order_by

Reported by: Ben Buchwald Owned by: Matthew Wilkes
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: josh.smeaton@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


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.

Change History (16)

comment:1 Changed 9 years ago by Josh Smeaton

Cc: josh.smeaton@… added
Triage Stage: UnreviewedAccepted
Version: 1.8master

This is the general problem that matches these specific tickets:

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.
  1. 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: 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.

comment:2 Changed 9 years ago by Matthew 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.

comment:3 Changed 8 years ago by Artur Smęt

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?

comment:4 Changed 8 years ago by Josh Smeaton

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.

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.

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:5 Changed 7 years ago by Matthew Wilkes

Owner: changed from nobody to Matthew Wilkes
Status: newassigned

I'm having a go at this between talks at PyCon.

comment:6 Changed 7 years ago by Tim Graham

WIP PR from Matthew

comment:7 Changed 7 years ago 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


I've worked out a similar work around of

MyModel.objects.all().annotate(some_field=RawSQL("metadata #> '{some}' #> '{field}'", [])).values("some_field")

comment:8 Changed 7 years ago 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 accurately represent what you want from JSON fields?

comment:9 Changed 6 years ago by Matthew Wilkes

Has patch: set
Triage Stage: AcceptedUnreviewed

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.

comment:10 Changed 6 years ago by Simon Charette

Triage Stage: UnreviewedAccepted

comment:11 Changed 6 years ago 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.

comment:12 Changed 6 years ago by Josh Smeaton

Needs documentation: set
Needs tests: set
Patch needs improvement: set

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.

comment:13 Changed 6 years ago by Matthew Wilkes

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

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.

comment:14 Changed 6 years ago 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.)

comment:15 Changed 6 years ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:16 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 2162f09:

Fixed #24747 -- Allowed transforms in QuerySet.order_by() and distinct(*fields).

Note: See TracTickets for help on using tickets.
Back to Top