From b2a4cff51bffdd8abd22d26dd6bcf66568a10e17 Mon Sep 17 00:00:00 2001 From: starocean999 <40539150+starocean999@users.noreply.github.com> Date: Wed, 17 Jul 2024 13:48:21 +0800 Subject: [PATCH] [fix](nereids)fix nullable property of ForEachCombinator (#37980) ## Proposed changes pick from master https://github.com/apache/doris/pull/37796 --- .../combinator/ForEachCombinator.java | 19 ++++++++++++++----- .../visitor/AggregateFunctionVisitor.java | 2 +- .../function_p0/test_agg_foreach_notnull.out | 6 ++++++ .../test_agg_foreach_notnull.groovy | 4 ++++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/ForEachCombinator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/ForEachCombinator.java index c2a50a328a759c..a6d011ff0fbbf4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/ForEachCombinator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/combinator/ForEachCombinator.java @@ -20,9 +20,9 @@ import org.apache.doris.catalog.FunctionSignature; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.functions.AggCombinerFunctionBuilder; -import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction; +import org.apache.doris.nereids.trees.expressions.functions.agg.NullableAggregateFunction; import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; import org.apache.doris.nereids.types.ArrayType; @@ -36,8 +36,8 @@ /** * combinator foreach */ -public class ForEachCombinator extends AggregateFunction - implements UnaryExpression, ExplicitlyCastableSignature, AlwaysNullable, Combinator { +public class ForEachCombinator extends NullableAggregateFunction + implements UnaryExpression, ExplicitlyCastableSignature, Combinator { private final AggregateFunction nested; @@ -45,7 +45,11 @@ public class ForEachCombinator extends AggregateFunction * constructor of ForEachCombinator */ public ForEachCombinator(List arguments, AggregateFunction nested) { - super(nested.getName() + AggCombinerFunctionBuilder.FOREACH_SUFFIX, arguments); + this(arguments, false, nested); + } + + public ForEachCombinator(List arguments, boolean alwaysNullable, AggregateFunction nested) { + super(nested.getName() + AggCombinerFunctionBuilder.FOREACH_SUFFIX, false, alwaysNullable, arguments); this.nested = Objects.requireNonNull(nested, "nested can not be null"); } @@ -56,7 +60,7 @@ public static ForEachCombinator create(AggregateFunction nested) { @Override public ForEachCombinator withChildren(List children) { - return new ForEachCombinator(children, nested); + return new ForEachCombinator(children, alwaysNullable, nested); } @Override @@ -88,4 +92,9 @@ public AggregateFunction getNestedFunction() { public AggregateFunction withDistinctAndChildren(boolean distinct, List children) { throw new UnsupportedOperationException("Unimplemented method 'withDistinctAndChildren'"); } + + @Override + public NullableAggregateFunction withAlwaysNullable(boolean alwaysNullable) { + return new ForEachCombinator(children, alwaysNullable, nested); + } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/AggregateFunctionVisitor.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/AggregateFunctionVisitor.java index 6037d7b7057563..21bc0bf924b4b2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/AggregateFunctionVisitor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/AggregateFunctionVisitor.java @@ -322,7 +322,7 @@ default R visitUnionCombinator(UnionCombinator combinator, C context) { } default R visitForEachCombinator(ForEachCombinator combinator, C context) { - return visitAggregateFunction(combinator, context); + return visitNullableAggregateFunction(combinator, context); } default R visitJavaUdaf(JavaUdaf javaUdaf, C context) { diff --git a/regression-test/data/function_p0/test_agg_foreach_notnull.out b/regression-test/data/function_p0/test_agg_foreach_notnull.out index c7d50a501cbf35..551074719e5489 100644 --- a/regression-test/data/function_p0/test_agg_foreach_notnull.out +++ b/regression-test/data/function_p0/test_agg_foreach_notnull.out @@ -1,4 +1,10 @@ -- This file is automatically generated. You should know what you did if you want to edit this +-- !select_sum_not_null -- +[1, 2, 3] +[20] +[100] +[null, 2] + -- !sql -- [1, 2, 3] [1, 2, 3] [100, 2, 3] [100, 2, 3] [40.333333333333336, 2, 3] [85.95867768595042, 2, 3] diff --git a/regression-test/suites/function_p0/test_agg_foreach_notnull.groovy b/regression-test/suites/function_p0/test_agg_foreach_notnull.groovy index 91f4ea902dd6a3..a8057f46c84d58 100644 --- a/regression-test/suites/function_p0/test_agg_foreach_notnull.groovy +++ b/regression-test/suites/function_p0/test_agg_foreach_notnull.groovy @@ -61,6 +61,10 @@ suite("test_agg_foreach_not_null") { (4,[null,2],[[2],null],[null,'c']); """ + qt_select_sum_not_null """ + select sum_foreach(a) from foreach_table_not_null group by id order by id; + """ + // this case also test combinator should be case-insensitive qt_sql """ select min_ForEach(a), min_by_foreach(a,a),max_foreach(a),max_by_foreach(a,a) , avg_foreach(a),avg_weighted_foreach(a,a) from foreach_table_not_null ;