This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 23f1fc52946 [fix](Nereids) not equals and hashCode should contains
generate flag (#31123) (#31455)
23f1fc52946 is described below
commit 23f1fc5294679418dd0a2c590e8e4ad0f86dd4b9
Author: morrySnow <[email protected]>
AuthorDate: Tue Feb 27 16:11:30 2024 +0800
[fix](Nereids) not equals and hashCode should contains generate flag
(#31123) (#31455)
pick from master #31123
commit id f74b138e3f4d60a2ea4218e9a418965dd7cb4172
---
.../nereids/rules/rewrite/EliminateNotNull.java | 11 ++++++---
.../nereids/rules/rewrite/EliminateOuterJoin.java | 9 +++-----
.../nereids/rules/rewrite/InferAggNotNull.java | 25 ++++++++++++++++----
.../nereids/rules/rewrite/InferFilterNotNull.java | 26 ++++++++++++++-------
.../nereids/trees/expressions/Expression.java | 2 --
.../doris/nereids/trees/expressions/Not.java | 27 +++++++++++++++++-----
.../apache/doris/nereids/util/ExpressionUtils.java | 14 ++++-------
.../java/org/apache/doris/qe/SessionVariable.java | 2 +-
.../nereids/rules/rewrite/InferAggNotNullTest.java | 4 +++-
9 files changed, 79 insertions(+), 41 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateNotNull.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateNotNull.java
index a4d8eca40e3..5bfc8778d2a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateNotNull.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateNotNull.java
@@ -53,19 +53,23 @@ public class EliminateNotNull implements RewriteRuleFactory
{
public List<Rule> buildRules() {
return ImmutableList.of(
logicalFilter()
- .when(filter ->
filter.getConjuncts().stream().anyMatch(expr -> expr.isGeneratedIsNotNull))
.thenApply(ctx -> {
LogicalFilter<Plan> filter = ctx.root;
List<Expression> predicates =
removeGeneratedNotNull(filter.getConjuncts(),
ctx.cascadesContext);
+ if (predicates.size() ==
filter.getConjuncts().size()) {
+ return null;
+ }
return
PlanUtils.filterOrSelf(ImmutableSet.copyOf(predicates), filter.child());
}).toRule(RuleType.ELIMINATE_NOT_NULL),
innerLogicalJoin()
- .when(join ->
join.getOtherJoinConjuncts().stream().anyMatch(expr ->
expr.isGeneratedIsNotNull))
.thenApply(ctx -> {
LogicalJoin<Plan, Plan> join = ctx.root;
List<Expression> newOtherJoinConjuncts =
removeGeneratedNotNull(
join.getOtherJoinConjuncts(),
ctx.cascadesContext);
+ if (newOtherJoinConjuncts.size() ==
join.getOtherJoinConjuncts().size()) {
+ return null;
+ }
return
join.withJoinConjuncts(join.getHashJoinConjuncts(), newOtherJoinConjuncts);
})
.toRule(RuleType.ELIMINATE_NOT_NULL)
@@ -81,7 +85,8 @@ public class EliminateNotNull implements RewriteRuleFactory {
Set<Expression> predicatesNotContainIsNotNull = Sets.newHashSet();
List<Slot> slotsFromIsNotNull = Lists.newArrayList();
exprs.stream()
- .filter(expr -> !expr.isGeneratedIsNotNull) // remove
generated `is not null`
+ .filter(expr -> !(expr instanceof Not)
+ || !((Not) expr).isGeneratedIsNotNull()) // remove
generated `is not null`
.forEach(expr -> {
Optional<Slot> notNullSlot = TypeUtils.isNotNull(expr);
if (notNullSlot.isPresent()) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoin.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoin.java
index c2dcafbee43..8c4069badc5 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoin.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoin.java
@@ -74,8 +74,7 @@ public class EliminateOuterJoin extends OneRewriteRuleFactory
{
boolean conjunctsChanged = false;
if (!notNullSlots.isEmpty()) {
for (Slot slot : notNullSlots) {
- Not isNotNull = new Not(new IsNull(slot));
- isNotNull.isGeneratedIsNotNull = true;
+ Not isNotNull = new Not(new IsNull(slot), true);
conjunctsChanged |= conjuncts.add(isNotNull);
}
}
@@ -134,13 +133,11 @@ public class EliminateOuterJoin extends
OneRewriteRuleFactory {
private boolean createIsNotNullIfNecessary(EqualPredicate swapedEqualTo,
Collection<Expression> container) {
boolean containerChanged = false;
if (swapedEqualTo.left().nullable()) {
- Not not = new Not(new IsNull(swapedEqualTo.left()));
- not.isGeneratedIsNotNull = true;
+ Not not = new Not(new IsNull(swapedEqualTo.left()), true);
containerChanged |= container.add(not);
}
if (swapedEqualTo.right().nullable()) {
- Not not = new Not(new IsNull(swapedEqualTo.right()));
- not.isGeneratedIsNotNull = true;
+ Not not = new Not(new IsNull(swapedEqualTo.right()), true);
containerChanged |= container.add(not);
}
return containerChanged;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
index 55bf93ca58c..e30190592a6 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNull.java
@@ -20,6 +20,7 @@ package org.apache.doris.nereids.rules.rewrite;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.Not;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.Avg;
import org.apache.doris.nereids.trees.expressions.functions.agg.Count;
@@ -32,6 +33,9 @@ import
org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.util.ExpressionUtils;
import org.apache.doris.nereids.util.PlanUtils;
+import com.google.common.collect.ImmutableSet;
+
+import java.util.Collections;
import java.util.Set;
import java.util.stream.Collectors;
@@ -55,12 +59,25 @@ public class InferAggNotNull extends OneRewriteRuleFactory {
LogicalAggregate<Plan> agg = ctx.root;
Set<Expression> exprs =
agg.getAggregateFunctions().stream().flatMap(f -> f.children().stream())
.collect(Collectors.toSet());
- Set<Expression> isNotNull =
ExpressionUtils.inferNotNull(exprs, ctx.cascadesContext);
- if (isNotNull.size() == 0 || (agg.child() instanceof
Filter && isNotNull.equals(
- ((Filter) agg.child()).getConjuncts()))) {
+ Set<Expression> isNotNulls =
ExpressionUtils.inferNotNull(exprs, ctx.cascadesContext);
+ Set<Expression> predicates = Collections.emptySet();
+ if ((agg.child() instanceof Filter)) {
+ predicates = ((Filter) agg.child()).getConjuncts();
+ }
+ ImmutableSet.Builder<Expression>
needGenerateNotNullsBuilder = ImmutableSet.builder();
+ for (Expression isNotNull : isNotNulls) {
+ if (!predicates.contains(isNotNull)) {
+ isNotNull = ((Not)
isNotNull).withGeneratedIsNotNull(true);
+ if (!predicates.contains(isNotNull)) {
+ needGenerateNotNullsBuilder.add(isNotNull);
+ }
+ }
+ }
+ Set<Expression> needGenerateNotNulls =
needGenerateNotNullsBuilder.build();
+ if (needGenerateNotNulls.isEmpty()) {
return null;
}
- return agg.withChildren(PlanUtils.filter(isNotNull,
agg.child()).get());
+ return
agg.withChildren(PlanUtils.filter(needGenerateNotNulls, agg.child()).get());
}).toRule(RuleType.INFER_AGG_NOT_NULL);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferFilterNotNull.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferFilterNotNull.java
index e166a1ac892..f19976cb753 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferFilterNotNull.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InferFilterNotNull.java
@@ -20,13 +20,14 @@ package org.apache.doris.nereids.rules.rewrite;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.Not;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
import org.apache.doris.nereids.util.ExpressionUtils;
import org.apache.doris.nereids.util.PlanUtils;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSet.Builder;
+import com.google.common.collect.Streams;
import java.util.Set;
@@ -43,18 +44,27 @@ public class InferFilterNotNull extends
OneRewriteRuleFactory {
@Override
public Rule build() {
return logicalFilter()
- .when(filter -> filter.getConjuncts().stream().noneMatch(expr ->
expr.isGeneratedIsNotNull))
+ .when(filter -> filter.getConjuncts().stream()
+ .filter(Not.class::isInstance)
+ .map(Not.class::cast)
+ .noneMatch(Not::isGeneratedIsNotNull))
.thenApply(ctx -> {
LogicalFilter<Plan> filter = ctx.root;
Set<Expression> predicates = filter.getConjuncts();
- Set<Expression> isNotNull =
ExpressionUtils.inferNotNull(predicates, ctx.cascadesContext);
- if (isNotNull.isEmpty() || predicates.containsAll(isNotNull)) {
+ Set<Expression> isNotNulls =
ExpressionUtils.inferNotNull(predicates, ctx.cascadesContext);
+ ImmutableSet.Builder<Expression> needGenerateNotNullsBuilder =
ImmutableSet.builder();
+ for (Expression isNotNull : isNotNulls) {
+ if (!predicates.contains(isNotNull)) {
+ needGenerateNotNullsBuilder.add(((Not)
isNotNull).withGeneratedIsNotNull(true));
+ }
+ }
+ Set<Expression> needGenerateNotNulls =
needGenerateNotNullsBuilder.build();
+ if (needGenerateNotNulls.isEmpty()) {
return null;
}
- Builder<Expression> builder =
ImmutableSet.<Expression>builder()
- .addAll(predicates)
- .addAll(isNotNull);
- return PlanUtils.filter(builder.build(), filter.child()).get();
+ Set<Expression> conjuncts =
Streams.concat(predicates.stream(), needGenerateNotNulls.stream())
+ .collect(ImmutableSet.toImmutableSet());
+ return PlanUtils.filter(conjuncts, filter.child()).get();
}).toRule(RuleType.INFER_FILTER_NOT_NULL);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java
index 7bc26140a0f..8eb89da772d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java
@@ -52,8 +52,6 @@ import java.util.stream.Collectors;
* Abstract class for all Expression in Nereids.
*/
public abstract class Expression extends AbstractTreeNode<Expression>
implements ExpressionTrait {
- // Mask this expression is generated by rule, should be removed.
- public boolean isGeneratedIsNotNull = false;
protected Expression(Expression... children) {
super(children);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Not.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Not.java
index f62b18a677c..a324cdca094 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Not.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Not.java
@@ -39,12 +39,24 @@ public class Not extends Expression implements
UnaryExpression, ExpectsInputType
public static final List<AbstractDataType> EXPECTS_INPUT_TYPES =
ImmutableList.of(BooleanType.INSTANCE);
+ private final boolean isGeneratedIsNotNull;
+
public Not(Expression child) {
+ this(child, false);
+ }
+
+ public Not(Expression child, boolean isGeneratedIsNotNull) {
super(ImmutableList.of(child));
+ this.isGeneratedIsNotNull = isGeneratedIsNotNull;
}
- private Not(List<Expression> child) {
+ private Not(List<Expression> child, boolean isGeneratedIsNotNull) {
super(child);
+ this.isGeneratedIsNotNull = isGeneratedIsNotNull;
+ }
+
+ public boolean isGeneratedIsNotNull() {
+ return isGeneratedIsNotNull;
}
@Override
@@ -71,12 +83,13 @@ public class Not extends Expression implements
UnaryExpression, ExpectsInputType
return false;
}
Not other = (Not) o;
- return Objects.equals(child(), other.child());
+ return Objects.equals(child(), other.child())
+ && isGeneratedIsNotNull == other.isGeneratedIsNotNull;
}
@Override
public int hashCode() {
- return child().hashCode();
+ return Objects.hash(child().hashCode(), isGeneratedIsNotNull);
}
@Override
@@ -92,9 +105,11 @@ public class Not extends Expression implements
UnaryExpression, ExpectsInputType
@Override
public Not withChildren(List<Expression> children) {
Preconditions.checkArgument(children.size() == 1);
- Not not = new Not(children);
- not.isGeneratedIsNotNull = this.isGeneratedIsNotNull;
- return not;
+ return new Not(children, isGeneratedIsNotNull);
+ }
+
+ public Not withGeneratedIsNotNull(boolean isGeneratedIsNotNull) {
+ return new Not(children, isGeneratedIsNotNull);
}
@Override
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
index 0c5faa20957..09f09e7cf17 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
@@ -368,11 +368,8 @@ public class ExpressionUtils {
*/
public static Set<Expression> inferNotNull(Set<Expression> predicates,
CascadesContext cascadesContext) {
return inferNotNullSlots(predicates, cascadesContext).stream()
- .map(slot -> {
- Not isNotNull = new Not(new IsNull(slot));
- isNotNull.isGeneratedIsNotNull = true;
- return isNotNull;
- }).collect(Collectors.toSet());
+ .map(slot -> new Not(new IsNull(slot), false))
+ .collect(Collectors.toSet());
}
/**
@@ -382,11 +379,8 @@ public class ExpressionUtils {
CascadesContext cascadesContext) {
return inferNotNullSlots(predicates, cascadesContext).stream()
.filter(slots::contains)
- .map(slot -> {
- Not isNotNull = new Not(new IsNull(slot));
- isNotNull.isGeneratedIsNotNull = true;
- return isNotNull;
- }).collect(Collectors.toSet());
+ .map(slot -> new Not(new IsNull(slot), true))
+ .collect(Collectors.toSet());
}
public static <E extends Expression> List<E> flatExpressions(List<List<E>>
expressions) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
index 2001dae3481..40cf53db0d7 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
@@ -2061,7 +2061,7 @@ public class SessionVariable implements Serializable,
Writable {
return forbidUnknownColStats;
}
- public void setForbidUnownColStats(boolean forbid) {
+ public void setForbidUnknownColStats(boolean forbid) {
forbidUnknownColStats = forbid;
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
index dcb9b801a9d..7d20c2f22a6 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferAggNotNullTest.java
@@ -18,6 +18,7 @@
package org.apache.doris.nereids.rules.rewrite;
import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.Not;
import org.apache.doris.nereids.trees.expressions.functions.agg.Count;
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
@@ -44,7 +45,8 @@ class InferAggNotNullTest implements
MemoPatternMatchSupported {
.applyTopDown(new InferAggNotNull())
.matches(
logicalAggregate(
- logicalFilter().when(filter ->
filter.getConjuncts().stream().allMatch(e -> e.isGeneratedIsNotNull))
+ logicalFilter().when(filter ->
filter.getConjuncts().stream()
+ .allMatch(e -> ((Not)
e).isGeneratedIsNotNull()))
)
);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
