#33304: Window(order_by) should allow usage of descending string syntax to be 
used
-------------------------------------+-------------------------------------
               Reporter:  Simon      |          Owner:  nobody
  Charette                           |
                   Type:             |         Status:  assigned
  Cleanup/optimization               |
              Component:  Database   |        Version:  dev
  layer (models, ORM)                |
               Severity:  Normal     |       Keywords:
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 The `QuerySet.order_by` and
 
[https://github.com/django/django/blob/a7e7043c8746933dafce652507d3b821801cdc7d/django/contrib/postgres/aggregates/mixins.py#L11-L14
 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

 {{{#!python
 class OrderBy(ExpressionList):
     template = 'ORDER BY %(expressions)'

     def __init__(self, *expressions, *extra):
         expressions = [
             (Sort(F(o[1:]), descending=True) if isinstance(o, str) and
 o[0] == '-' else o)
             for o 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.

-- 
Ticket URL: <https://code.djangoproject.com/ticket/33304>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/052.dc4e2dea003c5f445fe953aa217a30dd%40djangoproject.com.

Reply via email to