You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This also is a necessary (but not sufficient) step towards being able to use Exprs in sets and maps (still need to implement Ord and Hash) which might have helped @waynexia in #792 with CSE and would help with other algebriac transformations in the future.
Describe the solution you'd like impl PartialOrd for Expr
Describe alternatives you've considered
None
Additional context
None
The text was updated successfully, but these errors were encountered:
Thanks, @alamb. That's a good point to have PartialOrd on Expr.
For the CSE use case, it would be nice if we can compare and arrange Exprs in some kinds of order. By doing that we can recognize a+b and b+a are the same expression. And I think it can help other things too.
For the CSE use case, it would be nice if we can compare and arrange Exprs in some kinds of order. By doing that we can recognize a+b and b+a are the same expression. And I think it can help other things too.
That is a good point @waynexia -- I think one common approach is typically called "normalization" (aka rewrite all expressions so logically equivalent expressions like a+b and b+a end up with the same actual Expr form.
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
We want to ordering
Expr
s in a consistent way in our tests. See details on https://github.com/influxdata/influxdb_iox/pull/2564 in https://github.com/influxdata/influxdb_iox/pull/2564#discussion_r710817261This also is a necessary (but not sufficient) step towards being able to use
Expr
s in sets and maps (still need to implementOrd
andHash
) which might have helped @waynexia in #792 with CSE and would help with other algebriac transformations in the future.Describe the solution you'd like
impl PartialOrd
forExpr
Describe alternatives you've considered
None
Additional context
None
The text was updated successfully, but these errors were encountered: