xzj7019 commented on code in PR #40878:
URL: https://github.com/apache/doris/pull/40878#discussion_r1776495914
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferPredicates.java:
##########
@@ -156,18 +159,22 @@ private Set<Expression> getAllExpressions(Plan left, Plan
right, Optional<Expres
Set<Expression> baseExpressions = pullUpPredicates(left);
baseExpressions.addAll(pullUpPredicates(right));
condition.ifPresent(on ->
baseExpressions.addAll(ExpressionUtils.extractConjunction(on)));
- baseExpressions.addAll(PredicatePropagation.infer(baseExpressions));
- return baseExpressions;
+ return PredicateInferUtils.inferPredicate(baseExpressions);
}
private Set<Expression> pullUpPredicates(Plan plan) {
- return Sets.newHashSet(plan.accept(pollUpPredicates, null));
+ return Sets.newLinkedHashSet(plan.accept(pollUpPredicates, null));
Review Comment:
pls add a todo here to do the pre-sorting for the predicates.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferPredicates.java:
##########
@@ -105,12 +113,7 @@ public Plan visitLogicalFilter(LogicalFilter<? extends
Plan> filter, JobContext
filter = visitChildren(this, filter, context);
Set<Expression> filterPredicates = pullUpPredicates(filter);
filterPredicates.removeAll(pullUpPredicates(filter.child()));
- filter.getConjuncts().forEach(filterPredicates::remove);
- if (!filterPredicates.isEmpty()) {
- filterPredicates.addAll(filter.getConjuncts());
- return new LogicalFilter<>(ImmutableSet.copyOf(filterPredicates),
filter.child());
- }
- return filter;
+ return new LogicalFilter<>(ImmutableSet.copyOf(filterPredicates),
filter.child());
Review Comment:
does it means the original logical will miss some pullup and infer opt? does
the new code produce the redundant pullup-ed filters?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferPredicates.java:
##########
@@ -156,18 +159,22 @@ private Set<Expression> getAllExpressions(Plan left, Plan
right, Optional<Expres
Set<Expression> baseExpressions = pullUpPredicates(left);
baseExpressions.addAll(pullUpPredicates(right));
condition.ifPresent(on ->
baseExpressions.addAll(ExpressionUtils.extractConjunction(on)));
- baseExpressions.addAll(PredicatePropagation.infer(baseExpressions));
- return baseExpressions;
+ return PredicateInferUtils.inferPredicate(baseExpressions);
Review Comment:
seems that PredicatePropagation.infer is replaced by the new utils, can the
former one be removed? Or need to replace all reference of the old one with the
new util?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
