From 1cdc5736573e948d6a6b85edfb0c2fc085f49b10 Mon Sep 17 00:00:00 2001 From: yujun Date: Fri, 23 Jan 2026 18:47:18 +0800 Subject: [PATCH 1/3] remove rule EliminateNotNull --- .../doris/nereids/jobs/executor/Rewriter.java | 5 - .../apache/doris/nereids/rules/RuleType.java | 1 - .../mv/AbstractMaterializedViewRule.java | 9 -- .../expression/rules/RangeInference.java | 56 ++------ .../rules/expression/rules/SimplifyRange.java | 2 +- .../rules/rewrite/ConstantPropagation.java | 5 - .../rules/rewrite/EliminateNotNull.java | 127 ------------------ .../rules/rewrite/EliminateOuterJoin.java | 6 +- .../rules/rewrite/InferAggNotNull.java | 25 ++-- .../rules/rewrite/InferFilterNotNull.java | 24 +--- .../doris/nereids/trees/expressions/Not.java | 48 +------ .../plans/commands/DeleteFromCommand.java | 2 +- .../doris/nereids/util/ExpressionUtils.java | 10 +- .../doris/nereids/util/RelationUtil.java | 2 +- .../rewrite/ConstantPropagationTest.java | 6 +- .../rules/rewrite/EliminateOuterJoinTest.java | 3 - .../rules/rewrite/InferAggNotNullTest.java | 9 +- .../rules/rewrite/InferJoinNotNullTest.java | 1 - .../eliminate_outer_join.groovy | 2 +- 19 files changed, 50 insertions(+), 293 deletions(-) delete mode 100644 fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateNotNull.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java index a1c4504e8c538e..8a97ab52252e39 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java @@ -80,7 +80,6 @@ import org.apache.doris.nereids.rules.rewrite.EliminateJoinByUnique; import org.apache.doris.nereids.rules.rewrite.EliminateJoinCondition; import org.apache.doris.nereids.rules.rewrite.EliminateLimit; -import org.apache.doris.nereids.rules.rewrite.EliminateNotNull; import org.apache.doris.nereids.rules.rewrite.EliminateNullAwareLeftAntiJoin; import org.apache.doris.nereids.rules.rewrite.EliminateOrderByConstant; import org.apache.doris.nereids.rules.rewrite.EliminateOrderByKey; @@ -340,9 +339,7 @@ public class Rewriter extends AbstractBatchJobExecutor { topDown( new EliminateDedupJoinCondition() ), - // eliminate useless not null or inferred not null // TODO: wait InferPredicates to infer more not null. - bottomUp(new EliminateNotNull()), topDown(new ConvertInnerOrCrossJoin()) ), topic("Set operation optimization", @@ -577,9 +574,7 @@ public class Rewriter extends AbstractBatchJobExecutor { ) ), - // eliminate useless not null or inferred not null // TODO: wait InferPredicates to infer more not null. - bottomUp(new EliminateNotNull()), topDown(new ConvertInnerOrCrossJoin()) ), topic("Set operation optimization", diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java index a5f3d1612ef1e8..5f28761138de82 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java @@ -291,7 +291,6 @@ public enum RuleType { ELIMINATE_JOIN_CONDITION(RuleTypeClass.REWRITE), ELIMINATE_FILTER_ON_ONE_RELATION(RuleTypeClass.REWRITE), ELIMINATE_SEMI_JOIN(RuleTypeClass.REWRITE), - ELIMINATE_NOT_NULL(RuleTypeClass.REWRITE), ELIMINATE_UNNECESSARY_PROJECT(RuleTypeClass.REWRITE), RECORD_PLAN_FOR_MV_PRE_REWRITE(RuleTypeClass.REWRITE), ELIMINATE_OUTER_JOIN(RuleTypeClass.REWRITE), diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java index 036ffcd225177e..014e156ff6e10c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java @@ -44,7 +44,6 @@ import org.apache.doris.nereids.trees.expressions.ComparisonPredicate; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.NamedExpression; -import org.apache.doris.nereids.trees.expressions.Not; import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.expressions.SlotReference; import org.apache.doris.nereids.trees.expressions.functions.scalar.DateTrunc; @@ -866,14 +865,6 @@ private boolean containsNullRejectSlot(Set> requireNoNullableViewSlot, CascadesContext cascadesContext) { Set queryPulledUpPredicates = queryPredicates.stream() .flatMap(expr -> ExpressionUtils.extractConjunction(expr).stream()) - .map(expr -> { - // NOTICE inferNotNull generate Not with isGeneratedIsNotNull = false, - // so, we need set this flag to false before comparison. - if (expr instanceof Not) { - return ((Not) expr).withGeneratedIsNotNull(false); - } - return expr; - }) .collect(Collectors.toSet()); Set queryNullRejectPredicates = ExpressionUtils.inferNotNull(queryPulledUpPredicates, cascadesContext); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java index 5467de2a9f25d7..34a4bcb7be89a0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java @@ -164,7 +164,7 @@ public ValueDesc visitNot(Not not, ExpressionRewriteContext context) { if (childValue instanceof DiscreteValue) { return new NotDiscreteValue(context, childValue.getReference(), ((DiscreteValue) childValue).values); } else if (childValue instanceof IsNullValue) { - return new IsNotNullValue(context, childValue.getReference(), not); + return new IsNotNullValue(context, childValue.getReference()); } else { return new UnknownValue(context, not); } @@ -190,8 +190,7 @@ private ValueDesc processCompound(ExpressionRewriteContext context, List isNotNull = expression -> expression instanceof Not - && expression.child(0) instanceof IsNull - && !((Not) expression).isGeneratedIsNotNull(); + && expression.child(0) instanceof IsNull; for (Expression predicate : predicates) { hasNullExpression = hasNullExpression || predicate.isNullLiteral(); hasIsNullExpression = hasIsNullExpression || predicate instanceof IsNull; @@ -278,14 +277,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen // = (TA is not null or null) and (TA is not null) // = TA is not null // = IsNotNull(TA) - if (rangeValue.isRangeAll() && collector.isNotNullValueOpt.isPresent()) { - // Notice that if collector has only isGenerateNotNullValueOpt, we should not keep the rangeAll here - // for expression: (Not(IsNull(TA)) OR NULL) AND GeneratedNot(IsNull(TA)) - // will be converted to RangeAll(TA) AND IsNotNullValue(TA, generated=true) - // if we skip this RangeAll, the final result will be IsNotNullValue(TA, generated=true) - // then convert back to expression: GeneratedNot(IsNull(TA)), - // but later EliminateNotNull rule will remove this generated Not expression, - // then the final result will be TRUE, which is wrong. + if (rangeValue.isRangeAll() && collector.hasIsNotNullValue) { continue; } if (mergeRangeValueDesc == null) { @@ -366,7 +358,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen resultValues.add(new EmptyValue(context, reference)); } if (collector.hasIsNullValue) { - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { return new UnknownValue(context, BooleanLiteral.FALSE); } // nullable's EmptyValue have contains IsNull, no need to add @@ -374,12 +366,11 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen resultValues.add(new IsNullValue(context, reference)); } } - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { if (collector.hasEmptyValue) { return new UnknownValue(context, BooleanLiteral.FALSE); } - collector.isNotNullValueOpt.ifPresent(resultValues::add); - collector.isGenerateNotNullValueOpt.ifPresent(resultValues::add); + resultValues.add(new IsNotNullValue(context, reference)); } Optional shortCutResult = mergeCompoundValues(context, reference, resultValues, collector, true); if (shortCutResult.isPresent()) { @@ -397,7 +388,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen } private ValueDesc union(ExpressionRewriteContext context, Expression reference, ValueDescCollector collector) { - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { if (!collector.rangeValues.isEmpty() || !collector.discreteValues.isEmpty() || !collector.notDiscreteValues.isEmpty()) { @@ -471,12 +462,12 @@ private ValueDesc union(ExpressionRewriteContext context, Expression reference, } if (collector.hasIsNullValue) { - if (collector.hasIsNotNullValue() || hasRangeAll) { + if (collector.hasIsNotNullValue || hasRangeAll) { return new UnknownValue(context, BooleanLiteral.TRUE); } resultValues.add(new IsNullValue(context, reference)); } - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { if (collector.hasEmptyValue) { // EmptyValue(TA) or TA is not null // = TA is null and null or TA is not null @@ -484,8 +475,7 @@ private ValueDesc union(ExpressionRewriteContext context, Expression reference, // = RangeAll(TA) resultValues.add(new RangeValue(context, reference, Range.all())); } else { - collector.isNotNullValueOpt.ifPresent(resultValues::add); - collector.isGenerateNotNullValueOpt.ifPresent(resultValues::add); + resultValues.add(new IsNotNullValue(context, reference)); } } @@ -615,11 +605,8 @@ public interface ValueDescVisitor { } private static class ValueDescCollector implements ValueDescVisitor { - // generated not is null != not is null - Optional isNotNullValueOpt = Optional.empty(); - Optional isGenerateNotNullValueOpt = Optional.empty(); - boolean hasIsNullValue = false; + boolean hasIsNotNullValue = false; boolean hasEmptyValue = false; List rangeValues = Lists.newArrayList(); List discreteValues = Lists.newArrayList(); @@ -635,10 +622,6 @@ int size() { return rangeValues.size() + discreteValues.size() + compoundValues.size() + unknownValues.size(); } - boolean hasIsNotNullValue() { - return isNotNullValueOpt.isPresent() || isGenerateNotNullValueOpt.isPresent(); - } - @Override public Void visitEmptyValue(EmptyValue emptyValue, Void context) { hasEmptyValue = true; @@ -671,11 +654,7 @@ public Void visitIsNullValue(IsNullValue isNullValue, Void context) { @Override public Void visitIsNotNullValue(IsNotNullValue isNotNullValue, Void context) { - if (isNotNullValue.not.isGeneratedIsNotNull()) { - isGenerateNotNullValueOpt = Optional.of(isNotNullValue); - } else { - isNotNullValueOpt = Optional.of(isNotNullValue); - } + hasIsNotNullValue = true; return null; } @@ -1214,15 +1193,8 @@ protected UnionType getUnionType(ValueDesc other, int depth) { * a is not null */ public static class IsNotNullValue extends ValueDesc { - final Not not; - - public IsNotNullValue(ExpressionRewriteContext context, Expression reference, Not not) { + public IsNotNullValue(ExpressionRewriteContext context, Expression reference) { super(context, reference); - this.not = not; - } - - public Not getNotExpression() { - return this.not; } @Override @@ -1238,7 +1210,7 @@ protected boolean nullable() { @Override protected boolean containsAll(ValueDesc other, int depth) { if (other instanceof IsNotNullValue) { - return not.isGeneratedIsNotNull() == ((IsNotNullValue) other).not.isGeneratedIsNotNull(); + return true; } else if (other instanceof CompoundValue) { return ((CompoundValue) other).isContainedAllBy(this, depth); } else { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java index cd86283afe8ebe..db20fd71369f70 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java @@ -177,7 +177,7 @@ public Expression visitIsNullValue(IsNullValue value, Void context) { @Override public Expression visitIsNotNullValue(IsNotNullValue value, Void context) { - return value.getNotExpression(); + return new Not(new IsNull(value.getReference())); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java index 2f160aba3fced6..55abc121c831d1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java @@ -481,11 +481,6 @@ private boolean needReplaceWithConstant(Expression expression, Map 0 and a is not null` -> `a > 0` - * - `is not null` predicate is generated by `InferFilterNotNull` - */ -public class EliminateNotNull implements RewriteRuleFactory { - @Override - public List buildRules() { - return ImmutableList.of( - logicalFilter() - .when(EliminateNotNull::containsNot) - .thenApply(ctx -> { - LogicalFilter filter = ctx.root; - List 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(EliminateNotNull::containsNot) - .thenApply(ctx -> { - LogicalJoin join = ctx.root; - List newOtherJoinConjuncts = removeGeneratedNotNull( - join.getOtherJoinConjuncts(), ctx.cascadesContext); - if (newOtherJoinConjuncts.size() == join.getOtherJoinConjuncts().size()) { - return null; - } - return join.withJoinConjuncts(join.getHashJoinConjuncts(), newOtherJoinConjuncts, - join.getJoinReorderContext()); - }) - .toRule(RuleType.ELIMINATE_NOT_NULL) - ); - } - - private List removeGeneratedNotNull(Collection exprs, CascadesContext ctx) { - // Example: `id > 0 and id is not null and name is not null(generated)` - // predicatesNotContainIsNotNull: `id > 0` - // predicatesNotContainIsNotNull infer nonNullable slots: `id` - // slotsFromIsNotNull: `id`, `name` - // remove `name` (it's generated), remove `id` (because `id > 0` already contains it) - Set predicatesNotContainIsNotNull = Sets.newLinkedHashSet(); - List slotsFromIsNotNull = Lists.newArrayList(); - - for (Expression expr : exprs) { - // remove generated `is not null` - if (!(expr instanceof Not) || !((Not) expr).isGeneratedIsNotNull()) { - Optional notNullSlot = TypeUtils.isNotNull(expr); - if (notNullSlot.isPresent()) { - slotsFromIsNotNull.add(notNullSlot.get()); - } else { - predicatesNotContainIsNotNull.add(expr); - } - } - } - - Set inferNonNotSlots = ExpressionUtils.inferNotNullSlots( - predicatesNotContainIsNotNull, ctx); - - ImmutableSet.Builder keepIsNotNull - = ImmutableSet.builderWithExpectedSize(slotsFromIsNotNull.size()); - for (Slot slot : slotsFromIsNotNull) { - if (slot.nullable() && !inferNonNotSlots.contains(slot)) { - keepIsNotNull.add(new Not(new IsNull(slot))); - } - } - - // merge predicatesNotContainIsNotNull and keepIsNotNull into a new List - return ImmutableList.builder() - .addAll(predicatesNotContainIsNotNull) - .addAll(keepIsNotNull.build()) - .build(); - } - - private static boolean containsNot(Plan plan) { - for (Expression expression : plan.getExpressions()) { - if (expression.containsType(Not.class)) { - return true; - } - } - return false; - } -} 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 b8704f5610b9ea..db8e4ebbce515c 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 @@ -75,7 +75,7 @@ public Rule build() { boolean conjunctsChanged = false; if (!notNullSlots.isEmpty()) { for (Slot slot : notNullSlots) { - Not isNotNull = new Not(new IsNull(slot), true); + Not isNotNull = new Not(new IsNull(slot)); conjunctsChanged |= conjuncts.add(isNotNull); } } @@ -134,11 +134,11 @@ private JoinType tryEliminateOuterJoin(JoinType joinType, boolean canFilterLeftN private boolean createIsNotNullIfNecessary(EqualPredicate swapedEqualTo, Collection container) { boolean containerChanged = false; if (swapedEqualTo.left().nullable()) { - Not not = new Not(new IsNull(swapedEqualTo.left()), true); + Not not = new Not(new IsNull(swapedEqualTo.left())); containerChanged |= container.add(not); } if (swapedEqualTo.right().nullable()) { - Not not = new Not(new IsNull(swapedEqualTo.right()), true); + Not not = new Not(new IsNull(swapedEqualTo.right())); 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 e30190592a6b3b..ce331629907c4b 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,7 +20,6 @@ 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; @@ -30,8 +29,8 @@ import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.algebra.Filter; import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate; +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; @@ -46,7 +45,7 @@ public class InferAggNotNull extends OneRewriteRuleFactory { @Override public Rule build() { return logicalAggregate() - .when(agg -> agg.getGroupByExpressions().size() == 0) + .when(agg -> agg.getGroupByExpressions().isEmpty()) .when(agg -> agg.getAggregateFunctions().size() == 1) .when(agg -> { Set funcs = agg.getAggregateFunctions(); @@ -64,20 +63,16 @@ public Rule build() { if ((agg.child() instanceof Filter)) { predicates = ((Filter) agg.child()).getConjuncts(); } - ImmutableSet.Builder needGenerateNotNullsBuilder = ImmutableSet.builder(); - for (Expression isNotNull : isNotNulls) { - if (!predicates.contains(isNotNull)) { - isNotNull = ((Not) isNotNull).withGeneratedIsNotNull(true); - if (!predicates.contains(isNotNull)) { - needGenerateNotNullsBuilder.add(isNotNull); - } - } - } - Set needGenerateNotNulls = needGenerateNotNullsBuilder.build(); - if (needGenerateNotNulls.isEmpty()) { + if (predicates.containsAll(isNotNulls)) { return null; } - return agg.withChildren(PlanUtils.filter(needGenerateNotNulls, agg.child()).get()); + ImmutableSet.Builder newPredicateBuilder + = ImmutableSet.builderWithExpectedSize(predicates.size() + isNotNulls.size()); + newPredicateBuilder.addAll(predicates); + newPredicateBuilder.addAll(isNotNulls); + Plan newFilterChild = agg.child() instanceof Filter ? agg.child().child(0) : agg.child(); + LogicalFilter newFilter = new LogicalFilter<>(newPredicateBuilder.build(), newFilterChild); + return agg.withChildren(newFilter); }).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 002a40dd16c1ce..94102c47f39735 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,7 +20,6 @@ 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; @@ -44,34 +43,17 @@ public class InferFilterNotNull extends OneRewriteRuleFactory { @Override public Rule build() { return logicalFilter() - .when(filter -> { - for (Expression conjunct : filter.getConjuncts()) { - if (conjunct.containsType(Not.class) - && conjunct.anyMatch(n -> n instanceof Not && ((Not) n).isGeneratedIsNotNull())) { - return false; - } - } - return true; - }) .thenApply(ctx -> { LogicalFilter filter = ctx.root; Set predicates = filter.getConjuncts(); Set isNotNulls = ExpressionUtils.inferNotNull(predicates, ctx.cascadesContext); - Builder needGenerateNotNullsBuilder = ImmutableSet.builder(); - for (Expression isNotNull : isNotNulls) { - if (!predicates.contains(isNotNull)) { - needGenerateNotNullsBuilder.add(((Not) isNotNull).withGeneratedIsNotNull(true)); - } - } - Set needGenerateNotNulls = needGenerateNotNullsBuilder.build(); - if (needGenerateNotNulls.isEmpty()) { + if (predicates.containsAll(isNotNulls)) { return null; } - Builder conjuncts = ImmutableSet.builderWithExpectedSize( - predicates.size() + needGenerateNotNulls.size()); + predicates.size() + isNotNulls.size()); conjuncts.addAll(predicates); - conjuncts.addAll(needGenerateNotNulls); + conjuncts.addAll(isNotNulls); return PlanUtils.filter(conjuncts.build(), filter.child()).get(); }).toRule(RuleType.INFER_FILTER_NOT_NULL); } 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 e12276ff57fb4d..880279ee41e935 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 @@ -29,7 +29,6 @@ import com.google.common.collect.ImmutableList; import java.util.List; -import java.util.Objects; /** * Not expression: not a. @@ -38,27 +37,16 @@ public class Not extends Expression implements UnaryExpression, ExpectsInputType public static final List EXPECTS_INPUT_TYPES = ImmutableList.of(BooleanType.INSTANCE); - private final boolean isGeneratedIsNotNull; - public Not(Expression child) { - this(child, false); - } - - public Not(List child, boolean isGeneratedIsNotNull, boolean inferred) { - super(child, inferred); - this.isGeneratedIsNotNull = isGeneratedIsNotNull; + this(ImmutableList.of(child)); } - public Not(Expression child, boolean isGeneratedIsNotNull) { - this(ImmutableList.of(child), isGeneratedIsNotNull); - } - - private Not(List child, boolean isGeneratedIsNotNull) { - this(child, isGeneratedIsNotNull, false); + private Not(List child) { + this(child, false); } - public boolean isGeneratedIsNotNull() { - return isGeneratedIsNotNull; + private Not(List child, boolean inferred) { + super(child, inferred); } @Override @@ -76,24 +64,6 @@ public R accept(ExpressionVisitor visitor, C context) { return visitor.visitNot(this, context); } - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - Not other = (Not) o; - return Objects.equals(child(), other.child()) - && isGeneratedIsNotNull == other.isGeneratedIsNotNull; - } - - @Override - public int computeHashCode() { - return Objects.hash(child().hashCode(), isGeneratedIsNotNull); - } - @Override public String toString() { return "( not " + child().toString() + ")"; @@ -114,11 +84,7 @@ public String computeToSql() { @Override public Not withChildren(List children) { Preconditions.checkArgument(children.size() == 1); - return new Not(children, isGeneratedIsNotNull); - } - - public Not withGeneratedIsNotNull(boolean isGeneratedIsNotNull) { - return new Not(children, isGeneratedIsNotNull); + return new Not(children); } @Override @@ -128,6 +94,6 @@ public List expectedInputTypes() { @Override public Expression withInferred(boolean inferred) { - return new Not(this.children, false, inferred); + return new Not(this.children, inferred); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java index bc2bba1b32544a..d6364ea0715db0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java @@ -242,7 +242,7 @@ private void updateSessionVariableForDelete(SessionVariable sessionVariable) { new SetVar(SessionVariable.FORBID_UNKNOWN_COLUMN_STATS, new StringLiteral("false"))); // disable eliminate not null rule List disableRules = Lists.newArrayList( - RuleType.ELIMINATE_NOT_NULL.name(), RuleType.INFER_FILTER_NOT_NULL.name()); + RuleType.INFER_FILTER_NOT_NULL.name()); disableRules.addAll(sessionVariable.getDisableNereidsRuleNames()); VariableMgr.setVar(sessionVariable, new SetVar(SessionVariable.DISABLE_NEREIDS_RULES, 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 dadfc9fa0f406a..2af92339f68fb7 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 @@ -768,7 +768,7 @@ public static Set inferNotNullSlots(Set predicates, CascadesCo public static Set inferNotNull(Set predicates, CascadesContext cascadesContext) { ImmutableSet.Builder newPredicates = ImmutableSet.builderWithExpectedSize(predicates.size()); for (Slot slot : inferNotNullSlots(predicates, cascadesContext)) { - newPredicates.add(new Not(new IsNull(slot), false)); + newPredicates.add(new Not(new IsNull(slot))); } return newPredicates.build(); } @@ -781,18 +781,12 @@ public static Set inferNotNull(Set predicates, Set ImmutableSet.Builder newPredicates = ImmutableSet.builderWithExpectedSize(predicates.size()); for (Slot slot : inferNotNullSlots(predicates, cascadesContext)) { if (slots.contains(slot)) { - newPredicates.add(new Not(new IsNull(slot), true)); + newPredicates.add(new Not(new IsNull(slot))); } } return newPredicates.build(); } - public static boolean isGeneratedNotNull(Expression expression) { - return expression instanceof Not - && ((Not) expression).isGeneratedIsNotNull() - && ((Not) expression).child() instanceof IsNull; - } - /** flatExpressions */ public static List flatExpressions(List> expressionLists) { int num = 0; diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java index 22131ca6c1721b..b2656b0db6c26f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java @@ -60,7 +60,7 @@ public class RelationUtil { + "ELIMINATE_GROUP_BY_KEY_BY_UNIFORM, HAVING_TO_FILTER, ELIMINATE_GROUP_BY, SIMPLIFY_AGG_GROUP_BY, " + "MERGE_PERCENTILE_TO_ARRAY, VARIANT_SUB_PATH_PRUNING, INFER_PREDICATES, INFER_AGG_NOT_NULL, " + "INFER_SET_OPERATOR_DISTINCT, INFER_FILTER_NOT_NULL, INFER_JOIN_NOT_NULL, PUSH_DOWN_MAX_MIN_FILTER, " - + "ELIMINATE_SORT, ELIMINATE_AGGREGATE, ELIMINATE_LIMIT, ELIMINATE_SEMI_JOIN, ELIMINATE_NOT_NULL, " + + "ELIMINATE_SORT, ELIMINATE_AGGREGATE, ELIMINATE_LIMIT, ELIMINATE_SEMI_JOIN, " + "ELIMINATE_JOIN_BY_UK, ELIMINATE_JOIN_BY_FK, ELIMINATE_GROUP_BY_KEY, ELIMINATE_GROUP_BY_KEY_BY_UNIFORM, " + "ELIMINATE_FILTER_GROUP_BY_KEY"; diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java index 285e8313e6a48c..fa3c9a53e2a0ec 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java @@ -166,15 +166,11 @@ void testExpressionNotReplace() { assertRewrite("t.a = 1 and t.a = t.b", "a = 1 and b = 1"); assertRewrite("t1.a = 1 and t1.a = t2.b", "a = 1 and a = b and b = 1"); - // for `a is not null`, if this Not isGeneratedIsNotNull, then will not rewrite it SlotReference a = new SlotReference("a", IntegerType.INSTANCE, true); - Expression expr1 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a), false)); + Expression expr1 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a))); Expression rewrittenExpr1 = rewriteExpression(expr1, true); Expression expectExpr1 = new EqualTo(a, new IntegerLiteral(1)); Assertions.assertEquals(expectExpr1, rewrittenExpr1); - Expression expr2 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a), true)); - Expression rewrittenExpr2 = rewriteExpression(expr2, true); - Assertions.assertEquals(expr2, rewrittenExpr2); // for `a match_any xx`, don't replace it, because the match require left child is column, not literal SlotReference b = new SlotReference("b", StringType.INSTANCE, true); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java index f0034410163649..93b4f799d4e99b 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java @@ -57,7 +57,6 @@ void testEliminateLeft() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferFilterNotNull()) .applyTopDown(new EliminateOuterJoin()) - .applyTopDown(new EliminateNotNull()) .printlnTree() .matchesFromRoot( logicalFilter( @@ -76,7 +75,6 @@ void testEliminateRight() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferFilterNotNull()) .applyTopDown(new EliminateOuterJoin()) - .applyTopDown(new EliminateNotNull()) .printlnTree() .matchesFromRoot( logicalFilter( @@ -98,7 +96,6 @@ void testEliminateBoth() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferFilterNotNull()) .applyTopDown(new EliminateOuterJoin()) - .applyTopDown(new EliminateNotNull()) .printlnTree() .matchesFromRoot( logicalFilter( 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 7d20c2f22a6f2b..5b3b8d27da4cd5 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,7 +18,9 @@ package org.apache.doris.nereids.rules.rewrite; import org.apache.doris.nereids.trees.expressions.Alias; +import org.apache.doris.nereids.trees.expressions.IsNull; import org.apache.doris.nereids.trees.expressions.Not; +import org.apache.doris.nereids.trees.expressions.Slot; 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; @@ -36,17 +38,18 @@ class InferAggNotNullTest implements MemoPatternMatchSupported { @Test void testInfer() { + Slot slot = scan1.getOutput().get(1); LogicalPlan plan = new LogicalPlanBuilder(scan1) .aggGroupUsingIndex(ImmutableList.of(), - ImmutableList.of(new Alias(new Count(true, scan1.getOutput().get(1)), "dnt"))) + ImmutableList.of(new Alias(new Count(true, slot), "dnt"))) .build(); PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferAggNotNull()) .matches( logicalAggregate( - logicalFilter().when(filter -> filter.getConjuncts().stream() - .allMatch(e -> ((Not) e).isGeneratedIsNotNull())) + logicalFilter().when(filter -> filter.getConjuncts() + .contains(new Not(new IsNull(slot)))) ) ); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java index d963363a3793ab..887a9efca59e70 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java @@ -100,7 +100,6 @@ void testInferAndEliminate() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferJoinNotNull()) - .applyTopDown(new EliminateNotNull()) .matches( innerLogicalJoin( logicalOlapScan(), diff --git a/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy b/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy index 1129865a4f0cae..1818053774a2b6 100644 --- a/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy +++ b/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy @@ -23,7 +23,7 @@ suite("eliminate_outer_join") { sql "set enable_bucket_shuffle_join=false" sql "set runtime_filter_mode=OFF" sql "set ignore_shape_nodes='PhysicalDistribute, PhysicalProject'" - sql "set disable_nereids_rules='PRUNE_EMPTY_PARTITION,ELIMINATE_NOT_NULL'" + sql "set disable_nereids_rules='PRUNE_EMPTY_PARTITION'" String database = context.config.getDbNameByFile(context.file) sql "drop database if exists ${database}" From b749b1e7773db1b66ed41fb72eca77017a94900f Mon Sep 17 00:00:00 2001 From: yujun Date: Tue, 27 Jan 2026 16:24:49 +0800 Subject: [PATCH 2/3] Revert "remove rule EliminateNotNull" This reverts commit 1cdc5736573e948d6a6b85edfb0c2fc085f49b10. --- .../doris/nereids/jobs/executor/Rewriter.java | 5 + .../apache/doris/nereids/rules/RuleType.java | 1 + .../mv/AbstractMaterializedViewRule.java | 9 ++ .../expression/rules/RangeInference.java | 56 ++++++-- .../rules/expression/rules/SimplifyRange.java | 2 +- .../rules/rewrite/ConstantPropagation.java | 5 + .../rules/rewrite/EliminateNotNull.java | 127 ++++++++++++++++++ .../rules/rewrite/EliminateOuterJoin.java | 6 +- .../rules/rewrite/InferAggNotNull.java | 25 ++-- .../rules/rewrite/InferFilterNotNull.java | 24 +++- .../doris/nereids/trees/expressions/Not.java | 48 ++++++- .../plans/commands/DeleteFromCommand.java | 2 +- .../doris/nereids/util/ExpressionUtils.java | 10 +- .../doris/nereids/util/RelationUtil.java | 2 +- .../rewrite/ConstantPropagationTest.java | 6 +- .../rules/rewrite/EliminateOuterJoinTest.java | 3 + .../rules/rewrite/InferAggNotNullTest.java | 9 +- .../rules/rewrite/InferJoinNotNullTest.java | 1 + .../eliminate_outer_join.groovy | 2 +- 19 files changed, 293 insertions(+), 50 deletions(-) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateNotNull.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java index 8a97ab52252e39..a1c4504e8c538e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java @@ -80,6 +80,7 @@ import org.apache.doris.nereids.rules.rewrite.EliminateJoinByUnique; import org.apache.doris.nereids.rules.rewrite.EliminateJoinCondition; import org.apache.doris.nereids.rules.rewrite.EliminateLimit; +import org.apache.doris.nereids.rules.rewrite.EliminateNotNull; import org.apache.doris.nereids.rules.rewrite.EliminateNullAwareLeftAntiJoin; import org.apache.doris.nereids.rules.rewrite.EliminateOrderByConstant; import org.apache.doris.nereids.rules.rewrite.EliminateOrderByKey; @@ -339,7 +340,9 @@ public class Rewriter extends AbstractBatchJobExecutor { topDown( new EliminateDedupJoinCondition() ), + // eliminate useless not null or inferred not null // TODO: wait InferPredicates to infer more not null. + bottomUp(new EliminateNotNull()), topDown(new ConvertInnerOrCrossJoin()) ), topic("Set operation optimization", @@ -574,7 +577,9 @@ public class Rewriter extends AbstractBatchJobExecutor { ) ), + // eliminate useless not null or inferred not null // TODO: wait InferPredicates to infer more not null. + bottomUp(new EliminateNotNull()), topDown(new ConvertInnerOrCrossJoin()) ), topic("Set operation optimization", diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java index 5f28761138de82..a5f3d1612ef1e8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java @@ -291,6 +291,7 @@ public enum RuleType { ELIMINATE_JOIN_CONDITION(RuleTypeClass.REWRITE), ELIMINATE_FILTER_ON_ONE_RELATION(RuleTypeClass.REWRITE), ELIMINATE_SEMI_JOIN(RuleTypeClass.REWRITE), + ELIMINATE_NOT_NULL(RuleTypeClass.REWRITE), ELIMINATE_UNNECESSARY_PROJECT(RuleTypeClass.REWRITE), RECORD_PLAN_FOR_MV_PRE_REWRITE(RuleTypeClass.REWRITE), ELIMINATE_OUTER_JOIN(RuleTypeClass.REWRITE), diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java index 014e156ff6e10c..036ffcd225177e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java @@ -44,6 +44,7 @@ import org.apache.doris.nereids.trees.expressions.ComparisonPredicate; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.NamedExpression; +import org.apache.doris.nereids.trees.expressions.Not; import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.expressions.SlotReference; import org.apache.doris.nereids.trees.expressions.functions.scalar.DateTrunc; @@ -865,6 +866,14 @@ private boolean containsNullRejectSlot(Set> requireNoNullableViewSlot, CascadesContext cascadesContext) { Set queryPulledUpPredicates = queryPredicates.stream() .flatMap(expr -> ExpressionUtils.extractConjunction(expr).stream()) + .map(expr -> { + // NOTICE inferNotNull generate Not with isGeneratedIsNotNull = false, + // so, we need set this flag to false before comparison. + if (expr instanceof Not) { + return ((Not) expr).withGeneratedIsNotNull(false); + } + return expr; + }) .collect(Collectors.toSet()); Set queryNullRejectPredicates = ExpressionUtils.inferNotNull(queryPulledUpPredicates, cascadesContext); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java index 34a4bcb7be89a0..5467de2a9f25d7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java @@ -164,7 +164,7 @@ public ValueDesc visitNot(Not not, ExpressionRewriteContext context) { if (childValue instanceof DiscreteValue) { return new NotDiscreteValue(context, childValue.getReference(), ((DiscreteValue) childValue).values); } else if (childValue instanceof IsNullValue) { - return new IsNotNullValue(context, childValue.getReference()); + return new IsNotNullValue(context, childValue.getReference(), not); } else { return new UnknownValue(context, not); } @@ -190,7 +190,8 @@ private ValueDesc processCompound(ExpressionRewriteContext context, List isNotNull = expression -> expression instanceof Not - && expression.child(0) instanceof IsNull; + && expression.child(0) instanceof IsNull + && !((Not) expression).isGeneratedIsNotNull(); for (Expression predicate : predicates) { hasNullExpression = hasNullExpression || predicate.isNullLiteral(); hasIsNullExpression = hasIsNullExpression || predicate instanceof IsNull; @@ -277,7 +278,14 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen // = (TA is not null or null) and (TA is not null) // = TA is not null // = IsNotNull(TA) - if (rangeValue.isRangeAll() && collector.hasIsNotNullValue) { + if (rangeValue.isRangeAll() && collector.isNotNullValueOpt.isPresent()) { + // Notice that if collector has only isGenerateNotNullValueOpt, we should not keep the rangeAll here + // for expression: (Not(IsNull(TA)) OR NULL) AND GeneratedNot(IsNull(TA)) + // will be converted to RangeAll(TA) AND IsNotNullValue(TA, generated=true) + // if we skip this RangeAll, the final result will be IsNotNullValue(TA, generated=true) + // then convert back to expression: GeneratedNot(IsNull(TA)), + // but later EliminateNotNull rule will remove this generated Not expression, + // then the final result will be TRUE, which is wrong. continue; } if (mergeRangeValueDesc == null) { @@ -358,7 +366,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen resultValues.add(new EmptyValue(context, reference)); } if (collector.hasIsNullValue) { - if (collector.hasIsNotNullValue) { + if (collector.hasIsNotNullValue()) { return new UnknownValue(context, BooleanLiteral.FALSE); } // nullable's EmptyValue have contains IsNull, no need to add @@ -366,11 +374,12 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen resultValues.add(new IsNullValue(context, reference)); } } - if (collector.hasIsNotNullValue) { + if (collector.hasIsNotNullValue()) { if (collector.hasEmptyValue) { return new UnknownValue(context, BooleanLiteral.FALSE); } - resultValues.add(new IsNotNullValue(context, reference)); + collector.isNotNullValueOpt.ifPresent(resultValues::add); + collector.isGenerateNotNullValueOpt.ifPresent(resultValues::add); } Optional shortCutResult = mergeCompoundValues(context, reference, resultValues, collector, true); if (shortCutResult.isPresent()) { @@ -388,7 +397,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen } private ValueDesc union(ExpressionRewriteContext context, Expression reference, ValueDescCollector collector) { - if (collector.hasIsNotNullValue) { + if (collector.hasIsNotNullValue()) { if (!collector.rangeValues.isEmpty() || !collector.discreteValues.isEmpty() || !collector.notDiscreteValues.isEmpty()) { @@ -462,12 +471,12 @@ private ValueDesc union(ExpressionRewriteContext context, Expression reference, } if (collector.hasIsNullValue) { - if (collector.hasIsNotNullValue || hasRangeAll) { + if (collector.hasIsNotNullValue() || hasRangeAll) { return new UnknownValue(context, BooleanLiteral.TRUE); } resultValues.add(new IsNullValue(context, reference)); } - if (collector.hasIsNotNullValue) { + if (collector.hasIsNotNullValue()) { if (collector.hasEmptyValue) { // EmptyValue(TA) or TA is not null // = TA is null and null or TA is not null @@ -475,7 +484,8 @@ private ValueDesc union(ExpressionRewriteContext context, Expression reference, // = RangeAll(TA) resultValues.add(new RangeValue(context, reference, Range.all())); } else { - resultValues.add(new IsNotNullValue(context, reference)); + collector.isNotNullValueOpt.ifPresent(resultValues::add); + collector.isGenerateNotNullValueOpt.ifPresent(resultValues::add); } } @@ -605,8 +615,11 @@ public interface ValueDescVisitor { } private static class ValueDescCollector implements ValueDescVisitor { + // generated not is null != not is null + Optional isNotNullValueOpt = Optional.empty(); + Optional isGenerateNotNullValueOpt = Optional.empty(); + boolean hasIsNullValue = false; - boolean hasIsNotNullValue = false; boolean hasEmptyValue = false; List rangeValues = Lists.newArrayList(); List discreteValues = Lists.newArrayList(); @@ -622,6 +635,10 @@ int size() { return rangeValues.size() + discreteValues.size() + compoundValues.size() + unknownValues.size(); } + boolean hasIsNotNullValue() { + return isNotNullValueOpt.isPresent() || isGenerateNotNullValueOpt.isPresent(); + } + @Override public Void visitEmptyValue(EmptyValue emptyValue, Void context) { hasEmptyValue = true; @@ -654,7 +671,11 @@ public Void visitIsNullValue(IsNullValue isNullValue, Void context) { @Override public Void visitIsNotNullValue(IsNotNullValue isNotNullValue, Void context) { - hasIsNotNullValue = true; + if (isNotNullValue.not.isGeneratedIsNotNull()) { + isGenerateNotNullValueOpt = Optional.of(isNotNullValue); + } else { + isNotNullValueOpt = Optional.of(isNotNullValue); + } return null; } @@ -1193,8 +1214,15 @@ protected UnionType getUnionType(ValueDesc other, int depth) { * a is not null */ public static class IsNotNullValue extends ValueDesc { - public IsNotNullValue(ExpressionRewriteContext context, Expression reference) { + final Not not; + + public IsNotNullValue(ExpressionRewriteContext context, Expression reference, Not not) { super(context, reference); + this.not = not; + } + + public Not getNotExpression() { + return this.not; } @Override @@ -1210,7 +1238,7 @@ protected boolean nullable() { @Override protected boolean containsAll(ValueDesc other, int depth) { if (other instanceof IsNotNullValue) { - return true; + return not.isGeneratedIsNotNull() == ((IsNotNullValue) other).not.isGeneratedIsNotNull(); } else if (other instanceof CompoundValue) { return ((CompoundValue) other).isContainedAllBy(this, depth); } else { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java index db20fd71369f70..cd86283afe8ebe 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java @@ -177,7 +177,7 @@ public Expression visitIsNullValue(IsNullValue value, Void context) { @Override public Expression visitIsNotNullValue(IsNotNullValue value, Void context) { - return new Not(new IsNull(value.getReference())); + return value.getNotExpression(); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java index 55abc121c831d1..2f160aba3fced6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java @@ -481,6 +481,11 @@ private boolean needReplaceWithConstant(Expression expression, Map 0 and a is not null` -> `a > 0` + * - `is not null` predicate is generated by `InferFilterNotNull` + */ +public class EliminateNotNull implements RewriteRuleFactory { + @Override + public List buildRules() { + return ImmutableList.of( + logicalFilter() + .when(EliminateNotNull::containsNot) + .thenApply(ctx -> { + LogicalFilter filter = ctx.root; + List 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(EliminateNotNull::containsNot) + .thenApply(ctx -> { + LogicalJoin join = ctx.root; + List newOtherJoinConjuncts = removeGeneratedNotNull( + join.getOtherJoinConjuncts(), ctx.cascadesContext); + if (newOtherJoinConjuncts.size() == join.getOtherJoinConjuncts().size()) { + return null; + } + return join.withJoinConjuncts(join.getHashJoinConjuncts(), newOtherJoinConjuncts, + join.getJoinReorderContext()); + }) + .toRule(RuleType.ELIMINATE_NOT_NULL) + ); + } + + private List removeGeneratedNotNull(Collection exprs, CascadesContext ctx) { + // Example: `id > 0 and id is not null and name is not null(generated)` + // predicatesNotContainIsNotNull: `id > 0` + // predicatesNotContainIsNotNull infer nonNullable slots: `id` + // slotsFromIsNotNull: `id`, `name` + // remove `name` (it's generated), remove `id` (because `id > 0` already contains it) + Set predicatesNotContainIsNotNull = Sets.newLinkedHashSet(); + List slotsFromIsNotNull = Lists.newArrayList(); + + for (Expression expr : exprs) { + // remove generated `is not null` + if (!(expr instanceof Not) || !((Not) expr).isGeneratedIsNotNull()) { + Optional notNullSlot = TypeUtils.isNotNull(expr); + if (notNullSlot.isPresent()) { + slotsFromIsNotNull.add(notNullSlot.get()); + } else { + predicatesNotContainIsNotNull.add(expr); + } + } + } + + Set inferNonNotSlots = ExpressionUtils.inferNotNullSlots( + predicatesNotContainIsNotNull, ctx); + + ImmutableSet.Builder keepIsNotNull + = ImmutableSet.builderWithExpectedSize(slotsFromIsNotNull.size()); + for (Slot slot : slotsFromIsNotNull) { + if (slot.nullable() && !inferNonNotSlots.contains(slot)) { + keepIsNotNull.add(new Not(new IsNull(slot))); + } + } + + // merge predicatesNotContainIsNotNull and keepIsNotNull into a new List + return ImmutableList.builder() + .addAll(predicatesNotContainIsNotNull) + .addAll(keepIsNotNull.build()) + .build(); + } + + private static boolean containsNot(Plan plan) { + for (Expression expression : plan.getExpressions()) { + if (expression.containsType(Not.class)) { + return true; + } + } + return false; + } +} 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 db8e4ebbce515c..b8704f5610b9ea 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 @@ -75,7 +75,7 @@ public Rule build() { boolean conjunctsChanged = false; if (!notNullSlots.isEmpty()) { for (Slot slot : notNullSlots) { - Not isNotNull = new Not(new IsNull(slot)); + Not isNotNull = new Not(new IsNull(slot), true); conjunctsChanged |= conjuncts.add(isNotNull); } } @@ -134,11 +134,11 @@ private JoinType tryEliminateOuterJoin(JoinType joinType, boolean canFilterLeftN private boolean createIsNotNullIfNecessary(EqualPredicate swapedEqualTo, Collection container) { boolean containerChanged = false; if (swapedEqualTo.left().nullable()) { - Not not = new Not(new IsNull(swapedEqualTo.left())); + 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 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 ce331629907c4b..e30190592a6b3b 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 @@ 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; @@ -29,8 +30,8 @@ import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.algebra.Filter; import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate; -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; @@ -45,7 +46,7 @@ public class InferAggNotNull extends OneRewriteRuleFactory { @Override public Rule build() { return logicalAggregate() - .when(agg -> agg.getGroupByExpressions().isEmpty()) + .when(agg -> agg.getGroupByExpressions().size() == 0) .when(agg -> agg.getAggregateFunctions().size() == 1) .when(agg -> { Set funcs = agg.getAggregateFunctions(); @@ -63,16 +64,20 @@ public Rule build() { if ((agg.child() instanceof Filter)) { predicates = ((Filter) agg.child()).getConjuncts(); } - if (predicates.containsAll(isNotNulls)) { + ImmutableSet.Builder needGenerateNotNullsBuilder = ImmutableSet.builder(); + for (Expression isNotNull : isNotNulls) { + if (!predicates.contains(isNotNull)) { + isNotNull = ((Not) isNotNull).withGeneratedIsNotNull(true); + if (!predicates.contains(isNotNull)) { + needGenerateNotNullsBuilder.add(isNotNull); + } + } + } + Set needGenerateNotNulls = needGenerateNotNullsBuilder.build(); + if (needGenerateNotNulls.isEmpty()) { return null; } - ImmutableSet.Builder newPredicateBuilder - = ImmutableSet.builderWithExpectedSize(predicates.size() + isNotNulls.size()); - newPredicateBuilder.addAll(predicates); - newPredicateBuilder.addAll(isNotNulls); - Plan newFilterChild = agg.child() instanceof Filter ? agg.child().child(0) : agg.child(); - LogicalFilter newFilter = new LogicalFilter<>(newPredicateBuilder.build(), newFilterChild); - return agg.withChildren(newFilter); + 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 94102c47f39735..002a40dd16c1ce 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,6 +20,7 @@ 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; @@ -43,17 +44,34 @@ public class InferFilterNotNull extends OneRewriteRuleFactory { @Override public Rule build() { return logicalFilter() + .when(filter -> { + for (Expression conjunct : filter.getConjuncts()) { + if (conjunct.containsType(Not.class) + && conjunct.anyMatch(n -> n instanceof Not && ((Not) n).isGeneratedIsNotNull())) { + return false; + } + } + return true; + }) .thenApply(ctx -> { LogicalFilter filter = ctx.root; Set predicates = filter.getConjuncts(); Set isNotNulls = ExpressionUtils.inferNotNull(predicates, ctx.cascadesContext); - if (predicates.containsAll(isNotNulls)) { + Builder needGenerateNotNullsBuilder = ImmutableSet.builder(); + for (Expression isNotNull : isNotNulls) { + if (!predicates.contains(isNotNull)) { + needGenerateNotNullsBuilder.add(((Not) isNotNull).withGeneratedIsNotNull(true)); + } + } + Set needGenerateNotNulls = needGenerateNotNullsBuilder.build(); + if (needGenerateNotNulls.isEmpty()) { return null; } + Builder conjuncts = ImmutableSet.builderWithExpectedSize( - predicates.size() + isNotNulls.size()); + predicates.size() + needGenerateNotNulls.size()); conjuncts.addAll(predicates); - conjuncts.addAll(isNotNulls); + conjuncts.addAll(needGenerateNotNulls); return PlanUtils.filter(conjuncts.build(), filter.child()).get(); }).toRule(RuleType.INFER_FILTER_NOT_NULL); } 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 880279ee41e935..e12276ff57fb4d 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 @@ -29,6 +29,7 @@ import com.google.common.collect.ImmutableList; import java.util.List; +import java.util.Objects; /** * Not expression: not a. @@ -37,16 +38,27 @@ public class Not extends Expression implements UnaryExpression, ExpectsInputType public static final List EXPECTS_INPUT_TYPES = ImmutableList.of(BooleanType.INSTANCE); - public Not(Expression child) { - this(ImmutableList.of(child)); - } + private final boolean isGeneratedIsNotNull; - private Not(List child) { + public Not(Expression child) { this(child, false); } - private Not(List child, boolean inferred) { + public Not(List child, boolean isGeneratedIsNotNull, boolean inferred) { super(child, inferred); + this.isGeneratedIsNotNull = isGeneratedIsNotNull; + } + + public Not(Expression child, boolean isGeneratedIsNotNull) { + this(ImmutableList.of(child), isGeneratedIsNotNull); + } + + private Not(List child, boolean isGeneratedIsNotNull) { + this(child, isGeneratedIsNotNull, false); + } + + public boolean isGeneratedIsNotNull() { + return isGeneratedIsNotNull; } @Override @@ -64,6 +76,24 @@ public R accept(ExpressionVisitor visitor, C context) { return visitor.visitNot(this, context); } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Not other = (Not) o; + return Objects.equals(child(), other.child()) + && isGeneratedIsNotNull == other.isGeneratedIsNotNull; + } + + @Override + public int computeHashCode() { + return Objects.hash(child().hashCode(), isGeneratedIsNotNull); + } + @Override public String toString() { return "( not " + child().toString() + ")"; @@ -84,7 +114,11 @@ public String computeToSql() { @Override public Not withChildren(List children) { Preconditions.checkArgument(children.size() == 1); - return new Not(children); + return new Not(children, isGeneratedIsNotNull); + } + + public Not withGeneratedIsNotNull(boolean isGeneratedIsNotNull) { + return new Not(children, isGeneratedIsNotNull); } @Override @@ -94,6 +128,6 @@ public List expectedInputTypes() { @Override public Expression withInferred(boolean inferred) { - return new Not(this.children, inferred); + return new Not(this.children, false, inferred); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java index d6364ea0715db0..bc2bba1b32544a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java @@ -242,7 +242,7 @@ private void updateSessionVariableForDelete(SessionVariable sessionVariable) { new SetVar(SessionVariable.FORBID_UNKNOWN_COLUMN_STATS, new StringLiteral("false"))); // disable eliminate not null rule List disableRules = Lists.newArrayList( - RuleType.INFER_FILTER_NOT_NULL.name()); + RuleType.ELIMINATE_NOT_NULL.name(), RuleType.INFER_FILTER_NOT_NULL.name()); disableRules.addAll(sessionVariable.getDisableNereidsRuleNames()); VariableMgr.setVar(sessionVariable, new SetVar(SessionVariable.DISABLE_NEREIDS_RULES, 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 2af92339f68fb7..dadfc9fa0f406a 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 @@ -768,7 +768,7 @@ public static Set inferNotNullSlots(Set predicates, CascadesCo public static Set inferNotNull(Set predicates, CascadesContext cascadesContext) { ImmutableSet.Builder newPredicates = ImmutableSet.builderWithExpectedSize(predicates.size()); for (Slot slot : inferNotNullSlots(predicates, cascadesContext)) { - newPredicates.add(new Not(new IsNull(slot))); + newPredicates.add(new Not(new IsNull(slot), false)); } return newPredicates.build(); } @@ -781,12 +781,18 @@ public static Set inferNotNull(Set predicates, Set ImmutableSet.Builder newPredicates = ImmutableSet.builderWithExpectedSize(predicates.size()); for (Slot slot : inferNotNullSlots(predicates, cascadesContext)) { if (slots.contains(slot)) { - newPredicates.add(new Not(new IsNull(slot))); + newPredicates.add(new Not(new IsNull(slot), true)); } } return newPredicates.build(); } + public static boolean isGeneratedNotNull(Expression expression) { + return expression instanceof Not + && ((Not) expression).isGeneratedIsNotNull() + && ((Not) expression).child() instanceof IsNull; + } + /** flatExpressions */ public static List flatExpressions(List> expressionLists) { int num = 0; diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java index b2656b0db6c26f..22131ca6c1721b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/RelationUtil.java @@ -60,7 +60,7 @@ public class RelationUtil { + "ELIMINATE_GROUP_BY_KEY_BY_UNIFORM, HAVING_TO_FILTER, ELIMINATE_GROUP_BY, SIMPLIFY_AGG_GROUP_BY, " + "MERGE_PERCENTILE_TO_ARRAY, VARIANT_SUB_PATH_PRUNING, INFER_PREDICATES, INFER_AGG_NOT_NULL, " + "INFER_SET_OPERATOR_DISTINCT, INFER_FILTER_NOT_NULL, INFER_JOIN_NOT_NULL, PUSH_DOWN_MAX_MIN_FILTER, " - + "ELIMINATE_SORT, ELIMINATE_AGGREGATE, ELIMINATE_LIMIT, ELIMINATE_SEMI_JOIN, " + + "ELIMINATE_SORT, ELIMINATE_AGGREGATE, ELIMINATE_LIMIT, ELIMINATE_SEMI_JOIN, ELIMINATE_NOT_NULL, " + "ELIMINATE_JOIN_BY_UK, ELIMINATE_JOIN_BY_FK, ELIMINATE_GROUP_BY_KEY, ELIMINATE_GROUP_BY_KEY_BY_UNIFORM, " + "ELIMINATE_FILTER_GROUP_BY_KEY"; diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java index fa3c9a53e2a0ec..285e8313e6a48c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java @@ -166,11 +166,15 @@ void testExpressionNotReplace() { assertRewrite("t.a = 1 and t.a = t.b", "a = 1 and b = 1"); assertRewrite("t1.a = 1 and t1.a = t2.b", "a = 1 and a = b and b = 1"); + // for `a is not null`, if this Not isGeneratedIsNotNull, then will not rewrite it SlotReference a = new SlotReference("a", IntegerType.INSTANCE, true); - Expression expr1 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a))); + Expression expr1 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a), false)); Expression rewrittenExpr1 = rewriteExpression(expr1, true); Expression expectExpr1 = new EqualTo(a, new IntegerLiteral(1)); Assertions.assertEquals(expectExpr1, rewrittenExpr1); + Expression expr2 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a), true)); + Expression rewrittenExpr2 = rewriteExpression(expr2, true); + Assertions.assertEquals(expr2, rewrittenExpr2); // for `a match_any xx`, don't replace it, because the match require left child is column, not literal SlotReference b = new SlotReference("b", StringType.INSTANCE, true); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java index 93b4f799d4e99b..f0034410163649 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOuterJoinTest.java @@ -57,6 +57,7 @@ void testEliminateLeft() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferFilterNotNull()) .applyTopDown(new EliminateOuterJoin()) + .applyTopDown(new EliminateNotNull()) .printlnTree() .matchesFromRoot( logicalFilter( @@ -75,6 +76,7 @@ void testEliminateRight() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferFilterNotNull()) .applyTopDown(new EliminateOuterJoin()) + .applyTopDown(new EliminateNotNull()) .printlnTree() .matchesFromRoot( logicalFilter( @@ -96,6 +98,7 @@ void testEliminateBoth() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferFilterNotNull()) .applyTopDown(new EliminateOuterJoin()) + .applyTopDown(new EliminateNotNull()) .printlnTree() .matchesFromRoot( logicalFilter( 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 5b3b8d27da4cd5..7d20c2f22a6f2b 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,9 +18,7 @@ package org.apache.doris.nereids.rules.rewrite; import org.apache.doris.nereids.trees.expressions.Alias; -import org.apache.doris.nereids.trees.expressions.IsNull; import org.apache.doris.nereids.trees.expressions.Not; -import org.apache.doris.nereids.trees.expressions.Slot; 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; @@ -38,18 +36,17 @@ class InferAggNotNullTest implements MemoPatternMatchSupported { @Test void testInfer() { - Slot slot = scan1.getOutput().get(1); LogicalPlan plan = new LogicalPlanBuilder(scan1) .aggGroupUsingIndex(ImmutableList.of(), - ImmutableList.of(new Alias(new Count(true, slot), "dnt"))) + ImmutableList.of(new Alias(new Count(true, scan1.getOutput().get(1)), "dnt"))) .build(); PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferAggNotNull()) .matches( logicalAggregate( - logicalFilter().when(filter -> filter.getConjuncts() - .contains(new Not(new IsNull(slot)))) + logicalFilter().when(filter -> filter.getConjuncts().stream() + .allMatch(e -> ((Not) e).isGeneratedIsNotNull())) ) ); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java index 887a9efca59e70..d963363a3793ab 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/InferJoinNotNullTest.java @@ -100,6 +100,7 @@ void testInferAndEliminate() { PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferJoinNotNull()) + .applyTopDown(new EliminateNotNull()) .matches( innerLogicalJoin( logicalOlapScan(), diff --git a/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy b/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy index 1818053774a2b6..1129865a4f0cae 100644 --- a/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy +++ b/regression-test/suites/nereids_p0/eliminate_outer_join/eliminate_outer_join.groovy @@ -23,7 +23,7 @@ suite("eliminate_outer_join") { sql "set enable_bucket_shuffle_join=false" sql "set runtime_filter_mode=OFF" sql "set ignore_shape_nodes='PhysicalDistribute, PhysicalProject'" - sql "set disable_nereids_rules='PRUNE_EMPTY_PARTITION'" + sql "set disable_nereids_rules='PRUNE_EMPTY_PARTITION,ELIMINATE_NOT_NULL'" String database = context.config.getDbNameByFile(context.file) sql "drop database if exists ${database}" From ffd986485c061721abdc624809d95a1835323321 Mon Sep 17 00:00:00 2001 From: yujun Date: Tue, 27 Jan 2026 16:44:13 +0800 Subject: [PATCH 3/3] NOT remove isGenerated field --- .../mv/AbstractMaterializedViewRule.java | 9 --- .../expression/rules/RangeInference.java | 56 +++++-------------- .../rules/expression/rules/SimplifyRange.java | 2 +- .../rules/rewrite/ConstantPropagation.java | 5 -- .../rules/rewrite/EliminateNotNull.java | 19 +++---- .../rules/rewrite/EliminateOuterJoin.java | 6 +- .../rules/rewrite/InferAggNotNull.java | 25 ++++----- .../rules/rewrite/InferFilterNotNull.java | 24 +------- .../doris/nereids/trees/expressions/Not.java | 48 +++------------- .../doris/nereids/util/ExpressionUtils.java | 10 +--- .../rewrite/ConstantPropagationTest.java | 6 +- .../rules/rewrite/InferAggNotNullTest.java | 9 ++- 12 files changed, 55 insertions(+), 164 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java index 036ffcd225177e..014e156ff6e10c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewRule.java @@ -44,7 +44,6 @@ import org.apache.doris.nereids.trees.expressions.ComparisonPredicate; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.NamedExpression; -import org.apache.doris.nereids.trees.expressions.Not; import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.expressions.SlotReference; import org.apache.doris.nereids.trees.expressions.functions.scalar.DateTrunc; @@ -866,14 +865,6 @@ private boolean containsNullRejectSlot(Set> requireNoNullableViewSlot, CascadesContext cascadesContext) { Set queryPulledUpPredicates = queryPredicates.stream() .flatMap(expr -> ExpressionUtils.extractConjunction(expr).stream()) - .map(expr -> { - // NOTICE inferNotNull generate Not with isGeneratedIsNotNull = false, - // so, we need set this flag to false before comparison. - if (expr instanceof Not) { - return ((Not) expr).withGeneratedIsNotNull(false); - } - return expr; - }) .collect(Collectors.toSet()); Set queryNullRejectPredicates = ExpressionUtils.inferNotNull(queryPulledUpPredicates, cascadesContext); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java index 5467de2a9f25d7..34a4bcb7be89a0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/RangeInference.java @@ -164,7 +164,7 @@ public ValueDesc visitNot(Not not, ExpressionRewriteContext context) { if (childValue instanceof DiscreteValue) { return new NotDiscreteValue(context, childValue.getReference(), ((DiscreteValue) childValue).values); } else if (childValue instanceof IsNullValue) { - return new IsNotNullValue(context, childValue.getReference(), not); + return new IsNotNullValue(context, childValue.getReference()); } else { return new UnknownValue(context, not); } @@ -190,8 +190,7 @@ private ValueDesc processCompound(ExpressionRewriteContext context, List isNotNull = expression -> expression instanceof Not - && expression.child(0) instanceof IsNull - && !((Not) expression).isGeneratedIsNotNull(); + && expression.child(0) instanceof IsNull; for (Expression predicate : predicates) { hasNullExpression = hasNullExpression || predicate.isNullLiteral(); hasIsNullExpression = hasIsNullExpression || predicate instanceof IsNull; @@ -278,14 +277,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen // = (TA is not null or null) and (TA is not null) // = TA is not null // = IsNotNull(TA) - if (rangeValue.isRangeAll() && collector.isNotNullValueOpt.isPresent()) { - // Notice that if collector has only isGenerateNotNullValueOpt, we should not keep the rangeAll here - // for expression: (Not(IsNull(TA)) OR NULL) AND GeneratedNot(IsNull(TA)) - // will be converted to RangeAll(TA) AND IsNotNullValue(TA, generated=true) - // if we skip this RangeAll, the final result will be IsNotNullValue(TA, generated=true) - // then convert back to expression: GeneratedNot(IsNull(TA)), - // but later EliminateNotNull rule will remove this generated Not expression, - // then the final result will be TRUE, which is wrong. + if (rangeValue.isRangeAll() && collector.hasIsNotNullValue) { continue; } if (mergeRangeValueDesc == null) { @@ -366,7 +358,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen resultValues.add(new EmptyValue(context, reference)); } if (collector.hasIsNullValue) { - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { return new UnknownValue(context, BooleanLiteral.FALSE); } // nullable's EmptyValue have contains IsNull, no need to add @@ -374,12 +366,11 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen resultValues.add(new IsNullValue(context, reference)); } } - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { if (collector.hasEmptyValue) { return new UnknownValue(context, BooleanLiteral.FALSE); } - collector.isNotNullValueOpt.ifPresent(resultValues::add); - collector.isGenerateNotNullValueOpt.ifPresent(resultValues::add); + resultValues.add(new IsNotNullValue(context, reference)); } Optional shortCutResult = mergeCompoundValues(context, reference, resultValues, collector, true); if (shortCutResult.isPresent()) { @@ -397,7 +388,7 @@ private ValueDesc intersect(ExpressionRewriteContext context, Expression referen } private ValueDesc union(ExpressionRewriteContext context, Expression reference, ValueDescCollector collector) { - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { if (!collector.rangeValues.isEmpty() || !collector.discreteValues.isEmpty() || !collector.notDiscreteValues.isEmpty()) { @@ -471,12 +462,12 @@ private ValueDesc union(ExpressionRewriteContext context, Expression reference, } if (collector.hasIsNullValue) { - if (collector.hasIsNotNullValue() || hasRangeAll) { + if (collector.hasIsNotNullValue || hasRangeAll) { return new UnknownValue(context, BooleanLiteral.TRUE); } resultValues.add(new IsNullValue(context, reference)); } - if (collector.hasIsNotNullValue()) { + if (collector.hasIsNotNullValue) { if (collector.hasEmptyValue) { // EmptyValue(TA) or TA is not null // = TA is null and null or TA is not null @@ -484,8 +475,7 @@ private ValueDesc union(ExpressionRewriteContext context, Expression reference, // = RangeAll(TA) resultValues.add(new RangeValue(context, reference, Range.all())); } else { - collector.isNotNullValueOpt.ifPresent(resultValues::add); - collector.isGenerateNotNullValueOpt.ifPresent(resultValues::add); + resultValues.add(new IsNotNullValue(context, reference)); } } @@ -615,11 +605,8 @@ public interface ValueDescVisitor { } private static class ValueDescCollector implements ValueDescVisitor { - // generated not is null != not is null - Optional isNotNullValueOpt = Optional.empty(); - Optional isGenerateNotNullValueOpt = Optional.empty(); - boolean hasIsNullValue = false; + boolean hasIsNotNullValue = false; boolean hasEmptyValue = false; List rangeValues = Lists.newArrayList(); List discreteValues = Lists.newArrayList(); @@ -635,10 +622,6 @@ int size() { return rangeValues.size() + discreteValues.size() + compoundValues.size() + unknownValues.size(); } - boolean hasIsNotNullValue() { - return isNotNullValueOpt.isPresent() || isGenerateNotNullValueOpt.isPresent(); - } - @Override public Void visitEmptyValue(EmptyValue emptyValue, Void context) { hasEmptyValue = true; @@ -671,11 +654,7 @@ public Void visitIsNullValue(IsNullValue isNullValue, Void context) { @Override public Void visitIsNotNullValue(IsNotNullValue isNotNullValue, Void context) { - if (isNotNullValue.not.isGeneratedIsNotNull()) { - isGenerateNotNullValueOpt = Optional.of(isNotNullValue); - } else { - isNotNullValueOpt = Optional.of(isNotNullValue); - } + hasIsNotNullValue = true; return null; } @@ -1214,15 +1193,8 @@ protected UnionType getUnionType(ValueDesc other, int depth) { * a is not null */ public static class IsNotNullValue extends ValueDesc { - final Not not; - - public IsNotNullValue(ExpressionRewriteContext context, Expression reference, Not not) { + public IsNotNullValue(ExpressionRewriteContext context, Expression reference) { super(context, reference); - this.not = not; - } - - public Not getNotExpression() { - return this.not; } @Override @@ -1238,7 +1210,7 @@ protected boolean nullable() { @Override protected boolean containsAll(ValueDesc other, int depth) { if (other instanceof IsNotNullValue) { - return not.isGeneratedIsNotNull() == ((IsNotNullValue) other).not.isGeneratedIsNotNull(); + return true; } else if (other instanceof CompoundValue) { return ((CompoundValue) other).isContainedAllBy(this, depth); } else { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java index cd86283afe8ebe..db20fd71369f70 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyRange.java @@ -177,7 +177,7 @@ public Expression visitIsNullValue(IsNullValue value, Void context) { @Override public Expression visitIsNotNullValue(IsNotNullValue value, Void context) { - return value.getNotExpression(); + return new Not(new IsNull(value.getReference())); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java index 2f160aba3fced6..55abc121c831d1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java @@ -481,11 +481,6 @@ private boolean needReplaceWithConstant(Expression expression, Map buildRules() { .when(EliminateNotNull::containsNot) .thenApply(ctx -> { LogicalFilter filter = ctx.root; - List predicates = removeGeneratedNotNull(filter.getConjuncts(), + List predicates = removeNotNull(filter.getConjuncts(), ctx.cascadesContext); if (predicates.size() == filter.getConjuncts().size()) { return null; @@ -65,7 +65,7 @@ public List buildRules() { .when(EliminateNotNull::containsNot) .thenApply(ctx -> { LogicalJoin join = ctx.root; - List newOtherJoinConjuncts = removeGeneratedNotNull( + List newOtherJoinConjuncts = removeNotNull( join.getOtherJoinConjuncts(), ctx.cascadesContext); if (newOtherJoinConjuncts.size() == join.getOtherJoinConjuncts().size()) { return null; @@ -77,7 +77,7 @@ public List buildRules() { ); } - private List removeGeneratedNotNull(Collection exprs, CascadesContext ctx) { + private List removeNotNull(Collection exprs, CascadesContext ctx) { // Example: `id > 0 and id is not null and name is not null(generated)` // predicatesNotContainIsNotNull: `id > 0` // predicatesNotContainIsNotNull infer nonNullable slots: `id` @@ -87,14 +87,11 @@ private List removeGeneratedNotNull(Collection exprs, Ca List slotsFromIsNotNull = Lists.newArrayList(); for (Expression expr : exprs) { - // remove generated `is not null` - if (!(expr instanceof Not) || !((Not) expr).isGeneratedIsNotNull()) { - Optional notNullSlot = TypeUtils.isNotNull(expr); - if (notNullSlot.isPresent()) { - slotsFromIsNotNull.add(notNullSlot.get()); - } else { - predicatesNotContainIsNotNull.add(expr); - } + Optional notNullSlot = TypeUtils.isNotNull(expr); + if (notNullSlot.isPresent()) { + slotsFromIsNotNull.add(notNullSlot.get()); + } else { + predicatesNotContainIsNotNull.add(expr); } } 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 b8704f5610b9ea..db8e4ebbce515c 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 @@ -75,7 +75,7 @@ public Rule build() { boolean conjunctsChanged = false; if (!notNullSlots.isEmpty()) { for (Slot slot : notNullSlots) { - Not isNotNull = new Not(new IsNull(slot), true); + Not isNotNull = new Not(new IsNull(slot)); conjunctsChanged |= conjuncts.add(isNotNull); } } @@ -134,11 +134,11 @@ private JoinType tryEliminateOuterJoin(JoinType joinType, boolean canFilterLeftN private boolean createIsNotNullIfNecessary(EqualPredicate swapedEqualTo, Collection container) { boolean containerChanged = false; if (swapedEqualTo.left().nullable()) { - Not not = new Not(new IsNull(swapedEqualTo.left()), true); + Not not = new Not(new IsNull(swapedEqualTo.left())); containerChanged |= container.add(not); } if (swapedEqualTo.right().nullable()) { - Not not = new Not(new IsNull(swapedEqualTo.right()), true); + Not not = new Not(new IsNull(swapedEqualTo.right())); 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 e30190592a6b3b..ce331629907c4b 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,7 +20,6 @@ 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; @@ -30,8 +29,8 @@ import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.algebra.Filter; import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate; +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; @@ -46,7 +45,7 @@ public class InferAggNotNull extends OneRewriteRuleFactory { @Override public Rule build() { return logicalAggregate() - .when(agg -> agg.getGroupByExpressions().size() == 0) + .when(agg -> agg.getGroupByExpressions().isEmpty()) .when(agg -> agg.getAggregateFunctions().size() == 1) .when(agg -> { Set funcs = agg.getAggregateFunctions(); @@ -64,20 +63,16 @@ public Rule build() { if ((agg.child() instanceof Filter)) { predicates = ((Filter) agg.child()).getConjuncts(); } - ImmutableSet.Builder needGenerateNotNullsBuilder = ImmutableSet.builder(); - for (Expression isNotNull : isNotNulls) { - if (!predicates.contains(isNotNull)) { - isNotNull = ((Not) isNotNull).withGeneratedIsNotNull(true); - if (!predicates.contains(isNotNull)) { - needGenerateNotNullsBuilder.add(isNotNull); - } - } - } - Set needGenerateNotNulls = needGenerateNotNullsBuilder.build(); - if (needGenerateNotNulls.isEmpty()) { + if (predicates.containsAll(isNotNulls)) { return null; } - return agg.withChildren(PlanUtils.filter(needGenerateNotNulls, agg.child()).get()); + ImmutableSet.Builder newPredicateBuilder + = ImmutableSet.builderWithExpectedSize(predicates.size() + isNotNulls.size()); + newPredicateBuilder.addAll(predicates); + newPredicateBuilder.addAll(isNotNulls); + Plan newFilterChild = agg.child() instanceof Filter ? agg.child().child(0) : agg.child(); + LogicalFilter newFilter = new LogicalFilter<>(newPredicateBuilder.build(), newFilterChild); + return agg.withChildren(newFilter); }).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 002a40dd16c1ce..94102c47f39735 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,7 +20,6 @@ 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; @@ -44,34 +43,17 @@ public class InferFilterNotNull extends OneRewriteRuleFactory { @Override public Rule build() { return logicalFilter() - .when(filter -> { - for (Expression conjunct : filter.getConjuncts()) { - if (conjunct.containsType(Not.class) - && conjunct.anyMatch(n -> n instanceof Not && ((Not) n).isGeneratedIsNotNull())) { - return false; - } - } - return true; - }) .thenApply(ctx -> { LogicalFilter filter = ctx.root; Set predicates = filter.getConjuncts(); Set isNotNulls = ExpressionUtils.inferNotNull(predicates, ctx.cascadesContext); - Builder needGenerateNotNullsBuilder = ImmutableSet.builder(); - for (Expression isNotNull : isNotNulls) { - if (!predicates.contains(isNotNull)) { - needGenerateNotNullsBuilder.add(((Not) isNotNull).withGeneratedIsNotNull(true)); - } - } - Set needGenerateNotNulls = needGenerateNotNullsBuilder.build(); - if (needGenerateNotNulls.isEmpty()) { + if (predicates.containsAll(isNotNulls)) { return null; } - Builder conjuncts = ImmutableSet.builderWithExpectedSize( - predicates.size() + needGenerateNotNulls.size()); + predicates.size() + isNotNulls.size()); conjuncts.addAll(predicates); - conjuncts.addAll(needGenerateNotNulls); + conjuncts.addAll(isNotNulls); return PlanUtils.filter(conjuncts.build(), filter.child()).get(); }).toRule(RuleType.INFER_FILTER_NOT_NULL); } 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 e12276ff57fb4d..880279ee41e935 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 @@ -29,7 +29,6 @@ import com.google.common.collect.ImmutableList; import java.util.List; -import java.util.Objects; /** * Not expression: not a. @@ -38,27 +37,16 @@ public class Not extends Expression implements UnaryExpression, ExpectsInputType public static final List EXPECTS_INPUT_TYPES = ImmutableList.of(BooleanType.INSTANCE); - private final boolean isGeneratedIsNotNull; - public Not(Expression child) { - this(child, false); - } - - public Not(List child, boolean isGeneratedIsNotNull, boolean inferred) { - super(child, inferred); - this.isGeneratedIsNotNull = isGeneratedIsNotNull; + this(ImmutableList.of(child)); } - public Not(Expression child, boolean isGeneratedIsNotNull) { - this(ImmutableList.of(child), isGeneratedIsNotNull); - } - - private Not(List child, boolean isGeneratedIsNotNull) { - this(child, isGeneratedIsNotNull, false); + private Not(List child) { + this(child, false); } - public boolean isGeneratedIsNotNull() { - return isGeneratedIsNotNull; + private Not(List child, boolean inferred) { + super(child, inferred); } @Override @@ -76,24 +64,6 @@ public R accept(ExpressionVisitor visitor, C context) { return visitor.visitNot(this, context); } - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - Not other = (Not) o; - return Objects.equals(child(), other.child()) - && isGeneratedIsNotNull == other.isGeneratedIsNotNull; - } - - @Override - public int computeHashCode() { - return Objects.hash(child().hashCode(), isGeneratedIsNotNull); - } - @Override public String toString() { return "( not " + child().toString() + ")"; @@ -114,11 +84,7 @@ public String computeToSql() { @Override public Not withChildren(List children) { Preconditions.checkArgument(children.size() == 1); - return new Not(children, isGeneratedIsNotNull); - } - - public Not withGeneratedIsNotNull(boolean isGeneratedIsNotNull) { - return new Not(children, isGeneratedIsNotNull); + return new Not(children); } @Override @@ -128,6 +94,6 @@ public List expectedInputTypes() { @Override public Expression withInferred(boolean inferred) { - return new Not(this.children, false, inferred); + return new Not(this.children, inferred); } } 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 dadfc9fa0f406a..2af92339f68fb7 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 @@ -768,7 +768,7 @@ public static Set inferNotNullSlots(Set predicates, CascadesCo public static Set inferNotNull(Set predicates, CascadesContext cascadesContext) { ImmutableSet.Builder newPredicates = ImmutableSet.builderWithExpectedSize(predicates.size()); for (Slot slot : inferNotNullSlots(predicates, cascadesContext)) { - newPredicates.add(new Not(new IsNull(slot), false)); + newPredicates.add(new Not(new IsNull(slot))); } return newPredicates.build(); } @@ -781,18 +781,12 @@ public static Set inferNotNull(Set predicates, Set ImmutableSet.Builder newPredicates = ImmutableSet.builderWithExpectedSize(predicates.size()); for (Slot slot : inferNotNullSlots(predicates, cascadesContext)) { if (slots.contains(slot)) { - newPredicates.add(new Not(new IsNull(slot), true)); + newPredicates.add(new Not(new IsNull(slot))); } } return newPredicates.build(); } - public static boolean isGeneratedNotNull(Expression expression) { - return expression instanceof Not - && ((Not) expression).isGeneratedIsNotNull() - && ((Not) expression).child() instanceof IsNull; - } - /** flatExpressions */ public static List flatExpressions(List> expressionLists) { int num = 0; diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java index 285e8313e6a48c..fa3c9a53e2a0ec 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java @@ -166,15 +166,11 @@ void testExpressionNotReplace() { assertRewrite("t.a = 1 and t.a = t.b", "a = 1 and b = 1"); assertRewrite("t1.a = 1 and t1.a = t2.b", "a = 1 and a = b and b = 1"); - // for `a is not null`, if this Not isGeneratedIsNotNull, then will not rewrite it SlotReference a = new SlotReference("a", IntegerType.INSTANCE, true); - Expression expr1 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a), false)); + Expression expr1 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a))); Expression rewrittenExpr1 = rewriteExpression(expr1, true); Expression expectExpr1 = new EqualTo(a, new IntegerLiteral(1)); Assertions.assertEquals(expectExpr1, rewrittenExpr1); - Expression expr2 = ExpressionUtils.and(new EqualTo(a, new IntegerLiteral(1)), new Not(new IsNull(a), true)); - Expression rewrittenExpr2 = rewriteExpression(expr2, true); - Assertions.assertEquals(expr2, rewrittenExpr2); // for `a match_any xx`, don't replace it, because the match require left child is column, not literal SlotReference b = new SlotReference("b", StringType.INSTANCE, true); 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 7d20c2f22a6f2b..5b3b8d27da4cd5 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,7 +18,9 @@ package org.apache.doris.nereids.rules.rewrite; import org.apache.doris.nereids.trees.expressions.Alias; +import org.apache.doris.nereids.trees.expressions.IsNull; import org.apache.doris.nereids.trees.expressions.Not; +import org.apache.doris.nereids.trees.expressions.Slot; 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; @@ -36,17 +38,18 @@ class InferAggNotNullTest implements MemoPatternMatchSupported { @Test void testInfer() { + Slot slot = scan1.getOutput().get(1); LogicalPlan plan = new LogicalPlanBuilder(scan1) .aggGroupUsingIndex(ImmutableList.of(), - ImmutableList.of(new Alias(new Count(true, scan1.getOutput().get(1)), "dnt"))) + ImmutableList.of(new Alias(new Count(true, slot), "dnt"))) .build(); PlanChecker.from(MemoTestUtils.createConnectContext(), plan) .applyTopDown(new InferAggNotNull()) .matches( logicalAggregate( - logicalFilter().when(filter -> filter.getConjuncts().stream() - .allMatch(e -> ((Not) e).isGeneratedIsNotNull())) + logicalFilter().when(filter -> filter.getConjuncts() + .contains(new Not(new IsNull(slot)))) ) ); }