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]> 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 ). > 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]. > To post to this group, send email to [email protected]. > Visit this group at 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. > -- 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]. To post to this group, send email to [email protected]. Visit this group at 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. For more options, visit https://groups.google.com/d/optout.
