> 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 django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. 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. For more options, visit https://groups.google.com/d/optout.
