Agreed. It's worth noting that we could have a `Desc` operator
for awkward edge cases where __neg__ doesn't work well, and the
default implementation of __neg__ just returns Desc(self). I've
not completely considered all the possible implications here,
just throwing the idea around.
On 9 June 2014 01:53, Josh Smeaton <[email protected]
<javascript:>> wrote:
> That sounds like a workable approach. If I'm understanding
right, this means putting the ordering flag on the
ExpressionNode class so that all nodes will have this whether
or not they are to be used in an ordering context?
Yes, that had been what I was thinking, based upon the
implementation of PeeWee (
https://github.com/coleifer/peewee/blob/master/peewee.py#L292
<https://github.com/coleifer/peewee/blob/master/peewee.py#L292>
). At first it doesn't seem very nice; having an expression
containing properties regarding ordering or even aggregates
(like the current is_aggregate). But in my experience it has
turned out to be the nicest place to put that information,
that doesn't infect a large number of components. "An
expression can be ordered" makes sense.
> My only reservation about this when combined with __neg__
is that it means that certain mathematical expressions will
behave oddly
__neg__ isn't currently supported on expressions, so it has
no current meaning. It could be used in the mathematical
context though in the future, and would make sense.
> I think supporting this gives lots of potential problems
without much in the way of gain. Even the "correct" form
Coalesce('-title', '-description') just looks odd to me. The
DESC modifier applies to the expression as a whole, not to
any one field within it
Totally agree. Ordering applies to the expression, not any
one node within that expression. So we need to figure out how
best to support:
.order_by( F('field_a')+F('field_b') )
You can't apply ordering to field_a or field_b, because the
returned Expression is actually `+ (field_a, field_b)`.
Considering that, a wrapping `Desc(expression)` probably
makes a lot of sense, but has the following problems:
1. The syntax is somewhat ugly, but fits with the theme of
composable expressions.
2. If `Desc()` is just another expression, you'd need to
prevent it from being used in an `inner` context. It can only
be a wrapper. I think documenting this limitation is fine -
but if there was a way to throw a useful error (rather than
the database error that would surely follow), it'd be nice.
3. order_by(F('-field_a')) wouldn't be possible - there's no
way for the `F()` to push the `Desc()` to the top of the
tree. Since this isn't already supported anyway, I don't
think it's that big of a deal.
4. order_by('-field_a') should produce: Desc(F('field_a')),
but I think that'd be very easy. Expressions should be the
only supported API for order_by, except a string referencing
a field (like it is now) that is internally converted to an
Expression. I do the same thing with Aggregates right now.
> An alternative is to use ~ instead of - meaning inverse
instead of negative. This might be more appropriate (but then
is ~LowerCase not UpperCase?).
I don't think anyone would ever expect ~LowerCase to mean
UpperCase, but it is still somewhat confusing syntax. ~ is
mostly used in Q() objects to mean NOT, and could be very
confusing. We also run into similar issues with the
`order_by( F()+F() )` use case above - applying a unary
operator to either of those objects won't actually result in
any ordering being applied.
Josh
On Monday, 9 June 2014 02:07:42 UTC+10, Tim Martin wrote:
On Sunday, 8 June 2014 13:24:01 UTC+1, Josh Smeaton wrote:
I've thought about this previously, and what I had in
mind was:
- Expressions should support an ordering parameter of
some kind in the constructor, and save it to an
ordering property. Default to ASC
- If we go with Marc's suggestion, the __neg__ can
set the above property. This would look very
consistent with the existing ordering api.
- F() objects should parse their field argument, and
internally set their ordering attribute appropriately
-> F('-title') should strip the `-` and set a DESC
ordering.
- Since expressions can be nested, you can't return
ASC/DESC from the as_sql() method, otherwise you'd
end up with weirdness like: Coalesce(field_a ASC,
field_b ASC) instead of Coalesce(field_a, field_b)
ASC. Therefore, the order_by machinery should call a
`get_ordering()` (or something named similarly) and
generate the ASC/DESC token accordingly.
That sounds like a workable approach. If I'm
understanding right, this means putting the ordering flag
on the ExpressionNode class so that all nodes will have
this whether or not they are to be used in an ordering
context?
My only reservation about this when combined with __neg__
is that it means that certain mathematical expressions
will behave oddly. For example, you might want to do
something like
Accounts.objects.filter(balance__lt=-F('overdraft_limit'))
but this will end up flipping the ordering rather than
doing a mathematical negative. Is this dealt with by
putting logic into the SQLEvaluator so that it knows
whether it's evaluating in an ordering context or a
SELECT or WHERE clause context and does the right thing?
Usage:
Article.objects.order_by(LowerCase('title',
ordering='-'))
Article.objects.order_by(-LowerCase('title')) # I
prefer this form
Article.objects.order_by('-title') and
Article.objects.order_by(F('-title')) # should
be equivalent (a string argument should be
converted to an F())
I'm unsure whether or not expressions should inspect
their "children" for an ordering if one isn't
supplied in the outer-level:
Article.objects.order_by(LowerCase('-title'))
Should the Lowercase try to resolve the ordering of
`title` and use it as it's own? I don't think so,
because it could lead to weird conflicts where
multiple arguments define different ordering:
Article.objects.order_by(Coalesce('-title',
'+description')) # is ASC or DESC used? Possibly
confusing
Thoughts?
I think supporting this gives lots of potential problems
without much in the way of gain. Even the "correct" form
Coalesce('-title', '-description') just looks odd to me.
The DESC modifier applies to the expression as a whole,
not to any one field within it.
Tim
--
You received this message because you are subscribed to the
Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from
it, send an email to [email protected]
<javascript:>.
To post to this group, send email to
[email protected] <javascript:>.
Visit this group at
http://groups.google.com/group/django-developers
<http://groups.google.com/group/django-developers>.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/6365dba1-2dfc-469d-988b-3cb800cc0f76%40googlegroups.com
<https://groups.google.com/d/msgid/django-developers/6365dba1-2dfc-469d-988b-3cb800cc0f76%40googlegroups.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout
<https://groups.google.com/d/optout>.
--
You received this message because you are subscribed to the
Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it,
send an email to [email protected] <javascript:>.
To post to this group, send email to [email protected]
<javascript:>.
Visit this group at
http://groups.google.com/group/django-developers
<http://groups.google.com/group/django-developers>.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/CAMwjO1E36SeDW4R_jQOcw%3DkGV%2BwG5uaE%3D1Vif5DyDgwJY3ikcA%40mail.gmail.com
<https://groups.google.com/d/msgid/django-developers/CAMwjO1E36SeDW4R_jQOcw%3DkGV%2BwG5uaE%3D1Vif5DyDgwJY3ikcA%40mail.gmail.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout
<https://groups.google.com/d/optout>.
