ozankabak commented on code in PR #10434:
URL: https://github.com/apache/datafusion/pull/10434#discussion_r1595444209
##########
datafusion/physical-expr/src/equivalence/properties.rs:
##########
@@ -198,6 +198,61 @@ impl EquivalenceProperties {
left: &Arc<dyn PhysicalExpr>,
right: &Arc<dyn PhysicalExpr>,
) {
+ // Discover new constants in the light of new ordering
+ if self.is_expr_constant(left) {
+ // Left expression is constant, add right as constant
+ if !physical_exprs_contains(&self.constants, right) {
+ self.constants.push(right.clone());
+ }
+ } else if self.is_expr_constant(right) {
+ // Right expression is constant, add left as constant
+ if !physical_exprs_contains(&self.constants, left) {
+ self.constants.push(left.clone());
+ }
+ }
+
+ // Discover new valid orderings in the light of new equality
+ // See issue: https://github.com/apache/datafusion/issues/9812 for
rationale
Review Comment:
```suggestion
// Discover new valid orderings in light of the new equality. For a
discussion, see:
// https://github.com/apache/datafusion/issues/9812
```
##########
datafusion/physical-expr/src/equivalence/properties.rs:
##########
@@ -198,6 +198,61 @@ impl EquivalenceProperties {
left: &Arc<dyn PhysicalExpr>,
right: &Arc<dyn PhysicalExpr>,
) {
+ // Discover new constants in the light of new ordering
Review Comment:
```suggestion
// Discover new constants in light of new the equality:
```
##########
datafusion/physical-expr/src/equivalence/mod.rs:
##########
@@ -47,48 +46,8 @@ pub fn collapse_lex_req(input: LexRequirement) ->
LexRequirement {
output.push(item);
}
}
- collapse_monotonic_lex_req(output)
-}
-
-/// This function constructs a normalized [`LexRequirement`] by filtering out
entries
-/// that are ordered if the next entry is.
-/// Used in `collapse_lex_req`
-fn collapse_monotonic_lex_req(input: LexRequirement) -> LexRequirement {
- input
- .iter()
- .enumerate()
- .filter_map(|(i, item)| {
- // If it's the last entry, there is no next entry
- if i == input.len() - 1 {
- return Some(item);
- }
- let next_expr = &input[i + 1];
-
- // Only handle expressions with exactly one child
- // TODO: it should be possible to handle expressions orderings
f(a, b, c), a, b, c
- // if f is monotonic in all arguments
- if !(item.expr.children().len() == 1
- && item.expr.children()[0].eq(&next_expr.expr))
- {
- return Some(item);
- }
-
- let opts = match next_expr.options {
- None => return Some(item),
- Some(opts) => opts,
- };
-
- if item.options.map(SortProperties::Ordered)
- ==
Some(item.expr.get_ordering(&[SortProperties::Ordered(opts)]))
- {
- // Remove the redundant sort
- return None;
- }
-
- Some(item)
- })
- .cloned()
- .collect::<Vec<_>>()
+ // collapse_monotonic_lex_req(output)
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
