You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "morrySnow (via GitHub)" <gi...@apache.org> on 2025/05/13 08:21:06 UTC
Re: [PR] [opt](nereids) extract common factor run recursively for the common part [doris]
morrySnow commented on code in PR #49774:
URL: https://github.com/apache/doris/pull/49774#discussion_r2086141562
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/ExtractCommonFactorRule.java:
##########
@@ -87,17 +88,16 @@ private static Expression extractCommonFactor(CompoundPredicate originExpr) {
// combine and delete some boolean literal predicate
// e.g. (a and true) -> true
Review Comment:
this comment is wrong? a and true -> a?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/ExtractCommonFactorRule.java:
##########
@@ -144,22 +138,6 @@ private static Expression extractCommonFactors(CompoundPredicate originPredicate
LinkedHashMap<Set<Integer>, Set<Expression>> commonFactorPartitions
= partitionByMostCommonFactors(commonFactorToPartIds);
- int extractedExpressionNum = 0;
- for (Set<Expression> exprs : commonFactorPartitions.values()) {
- extractedExpressionNum += exprs.size();
- }
-
- // no any common factor
- if (commonFactorPartitions.entrySet().iterator().next().getKey().size() <= 1
- && !(originPredicate.getWidth() > leftDeapTreePredicate.getWidth())
- && originExpressionNum <= extractedExpressionNum) {
- // this condition is important because it can avoid deap loop:
- // origin originExpr: A = 1 and (B > 0 and B < 10)
- // after ExtractCommonFactorRule: (A = 1 and B > 0) and (B < 10) (left deap tree)
- // after SimplifyRange: A = 1 and (B > 0 and B < 10) (right deap tree)
- return originPredicate;
- }
-
// now we can do extract common factors for each part:
Review Comment:
how can we infer extractedExprs from originPredicate, initPartitions and commonFactorPartitions
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/ExtractCommonFactorRule.java:
##########
@@ -144,22 +138,6 @@ private static Expression extractCommonFactors(CompoundPredicate originPredicate
LinkedHashMap<Set<Integer>, Set<Expression>> commonFactorPartitions
= partitionByMostCommonFactors(commonFactorToPartIds);
- int extractedExpressionNum = 0;
- for (Set<Expression> exprs : commonFactorPartitions.values()) {
- extractedExpressionNum += exprs.size();
- }
-
- // no any common factor
- if (commonFactorPartitions.entrySet().iterator().next().getKey().size() <= 1
- && !(originPredicate.getWidth() > leftDeapTreePredicate.getWidth())
- && originExpressionNum <= extractedExpressionNum) {
- // this condition is important because it can avoid deap loop:
- // origin originExpr: A = 1 and (B > 0 and B < 10)
- // after ExtractCommonFactorRule: (A = 1 and B > 0) and (B < 10) (left deap tree)
- // after SimplifyRange: A = 1 and (B > 0 and B < 10) (right deap tree)
- return originPredicate;
- }
-
// now we can do extract common factors for each part:
// originPredicate: (a and (b and c)) and (b or c)
// leftDeapTreePredicate: ((a and b) and c) and (b or c)
Review Comment:
this comment should update?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/ExtractCommonFactorRule.java:
##########
@@ -144,22 +138,6 @@ private static Expression extractCommonFactors(CompoundPredicate originPredicate
LinkedHashMap<Set<Integer>, Set<Expression>> commonFactorPartitions
= partitionByMostCommonFactors(commonFactorToPartIds);
- int extractedExpressionNum = 0;
- for (Set<Expression> exprs : commonFactorPartitions.values()) {
- extractedExpressionNum += exprs.size();
- }
-
- // no any common factor
- if (commonFactorPartitions.entrySet().iterator().next().getKey().size() <= 1
- && !(originPredicate.getWidth() > leftDeapTreePredicate.getWidth())
- && originExpressionNum <= extractedExpressionNum) {
- // this condition is important because it can avoid deap loop:
- // origin originExpr: A = 1 and (B > 0 and B < 10)
- // after ExtractCommonFactorRule: (A = 1 and B > 0) and (B < 10) (left deap tree)
- // after SimplifyRange: A = 1 and (B > 0 and B < 10) (right deap tree)
- return originPredicate;
- }
-
// now we can do extract common factors for each part:
Review Comment:
the commonFactorPartitions in this comment is different in upper comment, according to upper comment, the commonFactorPartitions should be
```
{[1, 3]: [b], [2]: [c], [0]: [a]}
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org