Opened 3 years ago
Last modified 3 years ago
#33304 closed New feature
Window(order_by) should allow usage of descending string syntax to be used — at Version 2
Reported by: | Simon Charette | Owned by: | Simon Charette |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The QuerySet.order_by
and some aggregates ordering kwarg allows for the leading dash syntax to be used but Window.order_by
doesn't as it solely wraps the provided order_by
in ExpressionList(expressions=order_by)
.
This makes for an inconsistent API so I suggest we reuse the logic in OrderableAggMixin.__init__
in Window.__init__
As a related note it seems most of the logic of OrderableAggMixin
could be simplified by using ExpressionList
.
It's a shame that we used ordering
and not order_by
as a kwarg for OrderableAggMixin
as it's now inconsistent. Also not sure how much of a public API the OrderBy
expression is but I wish it was initially named Sort
(or Ordering
?) so that we could define
class OrderBy(ExpressionList): template = 'ORDER BY %(expressions)s' def __init__(self, *expressions, *extra): expressions = [ (Sort(F(expr[1:]), descending=True) if isinstance(expr, str) and expr[0] == '-' else expr) for expr in expressions ] super().__init__(*expressions, **extra)
And then simply use this abstraction in Window
and Postgres orderable aggregates.
Assigning to myself as I plan to have a look at this in next few days.
Change History (2)
comment:1 by , 3 years ago
Owner: | changed from | to
---|
comment:2 by , 3 years ago
Description: | modified (diff) |
---|