From b36a481d4af969ed51e8f34c8c9a6126a5e4d0d2 Mon Sep 17 00:00:00 2001 From: amorynan Date: Tue, 13 Aug 2024 18:00:49 +0800 Subject: [PATCH 1/3] fix array min/max func --- .../array/function_array_aggregation.cpp | 8 ++++- .../functions/scalar/ArrayMax.java | 23 +++++++++++-- .../functions/scalar/ArrayMin.java | 23 +++++++++++-- .../scalar_function/Array.groovy | 33 +++++++++++++++++++ .../test_array_functions_by_literal.groovy | 31 +++++++++++++++++ 5 files changed, 111 insertions(+), 7 deletions(-) diff --git a/be/src/vec/functions/array/function_array_aggregation.cpp b/be/src/vec/functions/array/function_array_aggregation.cpp index e8a2fd9e952ae8..d2edfe34fb63af 100644 --- a/be/src/vec/functions/array/function_array_aggregation.cpp +++ b/be/src/vec/functions/array/function_array_aggregation.cpp @@ -147,7 +147,13 @@ struct ArrayAggregateImpl { const DataTypeArray* data_type_array = static_cast(remove_nullable(arguments[0]).get()); auto function = Function::create(data_type_array->get_nested_type()); - return function->get_return_type(); + if (function) { + return function->get_return_type(); + } else { + throw doris::Exception(ErrorCode::INVALID_ARGUMENT, + "Unexpected type {} for aggregation {}", + data_type_array->get_nested_type()->get_name(), operation); + } } static Status execute(Block& block, const ColumnNumbers& arguments, size_t result, diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java index c1d0eff1b27514..5032f2b8eae532 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java @@ -24,9 +24,17 @@ 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; +import org.apache.doris.nereids.types.BigIntType; +import org.apache.doris.nereids.types.BooleanType; import org.apache.doris.nereids.types.DataType; -import org.apache.doris.nereids.types.coercion.AnyDataType; -import org.apache.doris.nereids.types.coercion.FollowToAnyDataType; +import org.apache.doris.nereids.types.DecimalV2Type; +import org.apache.doris.nereids.types.DecimalV3Type; +import org.apache.doris.nereids.types.DoubleType; +import org.apache.doris.nereids.types.FloatType; +import org.apache.doris.nereids.types.IntegerType; +import org.apache.doris.nereids.types.LargeIntType; +import org.apache.doris.nereids.types.SmallIntType; +import org.apache.doris.nereids.types.TinyIntType; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -40,7 +48,16 @@ public class ArrayMax extends ScalarFunction implements ExplicitlyCastableSignat UnaryExpression, AlwaysNullable { public static final List SIGNATURES = ImmutableList.of( - FunctionSignature.ret(new FollowToAnyDataType(0)).args(ArrayType.of(new AnyDataType(0))) + FunctionSignature.ret(BooleanType.INSTANCE).args(ArrayType.of(BooleanType.INSTANCE)), + FunctionSignature.ret(TinyIntType.INSTANCE).args(ArrayType.of(TinyIntType.INSTANCE)), + FunctionSignature.ret(SmallIntType.INSTANCE).args(ArrayType.of(SmallIntType.INSTANCE)), + FunctionSignature.ret(IntegerType.INSTANCE).args(ArrayType.of(IntegerType.INSTANCE)), + FunctionSignature.ret(BigIntType.INSTANCE).args(ArrayType.of(BigIntType.INSTANCE)), + FunctionSignature.ret(LargeIntType.INSTANCE).args(ArrayType.of(LargeIntType.INSTANCE)), + FunctionSignature.ret(DecimalV3Type.WILDCARD).args(ArrayType.of(DecimalV3Type.WILDCARD)), + FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(ArrayType.of(DecimalV2Type.SYSTEM_DEFAULT)), + FunctionSignature.ret(FloatType.INSTANCE).args(ArrayType.of(FloatType.INSTANCE)), + FunctionSignature.ret(DoubleType.INSTANCE).args(ArrayType.of(DoubleType.INSTANCE)) ); /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java index dbfba39a2c4d7f..0e64cd1b1e3b51 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java @@ -24,9 +24,17 @@ 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; +import org.apache.doris.nereids.types.BigIntType; +import org.apache.doris.nereids.types.BooleanType; import org.apache.doris.nereids.types.DataType; -import org.apache.doris.nereids.types.coercion.AnyDataType; -import org.apache.doris.nereids.types.coercion.FollowToAnyDataType; +import org.apache.doris.nereids.types.DecimalV2Type; +import org.apache.doris.nereids.types.DecimalV3Type; +import org.apache.doris.nereids.types.DoubleType; +import org.apache.doris.nereids.types.FloatType; +import org.apache.doris.nereids.types.IntegerType; +import org.apache.doris.nereids.types.LargeIntType; +import org.apache.doris.nereids.types.SmallIntType; +import org.apache.doris.nereids.types.TinyIntType; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -40,7 +48,16 @@ public class ArrayMin extends ScalarFunction implements ExplicitlyCastableSignat UnaryExpression, AlwaysNullable { public static final List SIGNATURES = ImmutableList.of( - FunctionSignature.ret(new FollowToAnyDataType(0)).args(ArrayType.of(new AnyDataType(0))) + FunctionSignature.ret(BooleanType.INSTANCE).args(ArrayType.of(BooleanType.INSTANCE)), + FunctionSignature.ret(TinyIntType.INSTANCE).args(ArrayType.of(TinyIntType.INSTANCE)), + FunctionSignature.ret(SmallIntType.INSTANCE).args(ArrayType.of(SmallIntType.INSTANCE)), + FunctionSignature.ret(IntegerType.INSTANCE).args(ArrayType.of(IntegerType.INSTANCE)), + FunctionSignature.ret(BigIntType.INSTANCE).args(ArrayType.of(BigIntType.INSTANCE)), + FunctionSignature.ret(LargeIntType.INSTANCE).args(ArrayType.of(LargeIntType.INSTANCE)), + FunctionSignature.ret(DecimalV3Type.WILDCARD).args(ArrayType.of(DecimalV3Type.WILDCARD)), + FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(ArrayType.of(DecimalV2Type.SYSTEM_DEFAULT)), + FunctionSignature.ret(FloatType.INSTANCE).args(ArrayType.of(FloatType.INSTANCE)), + FunctionSignature.ret(DoubleType.INSTANCE).args(ArrayType.of(DoubleType.INSTANCE)) ); /** diff --git a/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy b/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy index 506742588b4d1e..1c10896e9f1f35 100644 --- a/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy +++ b/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy @@ -1321,4 +1321,37 @@ suite("nereids_scalar_fn_Array") { exception("errCode = 2") } + sql """ set enable_fold_constant_by_be=true; """ + qt_array_empty_be """select array()""" + + // array_min/max with nested array for args + test { + sql "select array_min(array(1,2,3),array(4,5,6));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } + test { + sql "select array_max(array(1,2,3),array(4,5,6));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } + + test { + sql "select array_min(array(split_by_string('a,b,c',',')));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } + test { + sql "select array_max(array(split_by_string('a,b,c',',')));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } } diff --git a/regression-test/suites/query_p0/sql_functions/array_functions/test_array_functions_by_literal.groovy b/regression-test/suites/query_p0/sql_functions/array_functions/test_array_functions_by_literal.groovy index 81689b63359990..14c6ed5b925f7f 100644 --- a/regression-test/suites/query_p0/sql_functions/array_functions/test_array_functions_by_literal.groovy +++ b/regression-test/suites/query_p0/sql_functions/array_functions/test_array_functions_by_literal.groovy @@ -419,4 +419,35 @@ suite("test_array_functions_by_literal") { sql """select array_apply(split_by_string("amory,is,better,committing", ","), '!=', '');""" exception("No matching function") } + // array_min/max with nested array for args + test { + sql "select array_min(array(1,2,3),array(4,5,6));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } + test { + sql "select array_max(array(1,2,3),array(4,5,6));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } + + test { + sql "select array_min(array(split_by_string('a,b,c',',')));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } + test { + sql "select array_max(array(split_by_string('a,b,c',',')));" + check{result, exception, startTime, endTime -> + assertTrue(exception != null) + logger.info(exception.message) + } + } + } From e6cdcdfa1eff14106cd95b057b55a49b4c7c0538 Mon Sep 17 00:00:00 2001 From: amorynan Date: Tue, 13 Aug 2024 19:59:28 +0800 Subject: [PATCH 2/3] fix sign: --- .../functions/scalar/ArrayMax.java | 32 +++++++------------ .../functions/scalar/ArrayMin.java | 32 +++++++------------ .../functions/scalar/ArrayReverseSort.java | 10 ++++++ .../functions/scalar/ArraySort.java | 10 ++++++ 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java index 5032f2b8eae532..f8a292052156d5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMax.java @@ -18,23 +18,16 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Expression; 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.shape.UnaryExpression; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; import org.apache.doris.nereids.types.ArrayType; -import org.apache.doris.nereids.types.BigIntType; -import org.apache.doris.nereids.types.BooleanType; import org.apache.doris.nereids.types.DataType; -import org.apache.doris.nereids.types.DecimalV2Type; -import org.apache.doris.nereids.types.DecimalV3Type; -import org.apache.doris.nereids.types.DoubleType; -import org.apache.doris.nereids.types.FloatType; -import org.apache.doris.nereids.types.IntegerType; -import org.apache.doris.nereids.types.LargeIntType; -import org.apache.doris.nereids.types.SmallIntType; -import org.apache.doris.nereids.types.TinyIntType; +import org.apache.doris.nereids.types.coercion.AnyDataType; +import org.apache.doris.nereids.types.coercion.FollowToAnyDataType; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -48,16 +41,7 @@ public class ArrayMax extends ScalarFunction implements ExplicitlyCastableSignat UnaryExpression, AlwaysNullable { public static final List SIGNATURES = ImmutableList.of( - FunctionSignature.ret(BooleanType.INSTANCE).args(ArrayType.of(BooleanType.INSTANCE)), - FunctionSignature.ret(TinyIntType.INSTANCE).args(ArrayType.of(TinyIntType.INSTANCE)), - FunctionSignature.ret(SmallIntType.INSTANCE).args(ArrayType.of(SmallIntType.INSTANCE)), - FunctionSignature.ret(IntegerType.INSTANCE).args(ArrayType.of(IntegerType.INSTANCE)), - FunctionSignature.ret(BigIntType.INSTANCE).args(ArrayType.of(BigIntType.INSTANCE)), - FunctionSignature.ret(LargeIntType.INSTANCE).args(ArrayType.of(LargeIntType.INSTANCE)), - FunctionSignature.ret(DecimalV3Type.WILDCARD).args(ArrayType.of(DecimalV3Type.WILDCARD)), - FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(ArrayType.of(DecimalV2Type.SYSTEM_DEFAULT)), - FunctionSignature.ret(FloatType.INSTANCE).args(ArrayType.of(FloatType.INSTANCE)), - FunctionSignature.ret(DoubleType.INSTANCE).args(ArrayType.of(DoubleType.INSTANCE)) + FunctionSignature.ret(new FollowToAnyDataType(0)).args(ArrayType.of(new AnyDataType(0))) ); /** @@ -67,6 +51,14 @@ public ArrayMax(Expression arg) { super("array_max", arg); } + @Override + public void checkLegalityBeforeTypeCoercion() { + DataType argType = child().getDataType(); + if (((ArrayType) argType).getItemType().isComplexType()) { + throw new AnalysisException("array_max does not support complex types: " + toSql()); + } + } + @Override public DataType getDataType() { return ((ArrayType) (child().getDataType())).getItemType(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java index 0e64cd1b1e3b51..642b86f5752f2d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayMin.java @@ -18,23 +18,16 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Expression; 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.shape.UnaryExpression; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; import org.apache.doris.nereids.types.ArrayType; -import org.apache.doris.nereids.types.BigIntType; -import org.apache.doris.nereids.types.BooleanType; import org.apache.doris.nereids.types.DataType; -import org.apache.doris.nereids.types.DecimalV2Type; -import org.apache.doris.nereids.types.DecimalV3Type; -import org.apache.doris.nereids.types.DoubleType; -import org.apache.doris.nereids.types.FloatType; -import org.apache.doris.nereids.types.IntegerType; -import org.apache.doris.nereids.types.LargeIntType; -import org.apache.doris.nereids.types.SmallIntType; -import org.apache.doris.nereids.types.TinyIntType; +import org.apache.doris.nereids.types.coercion.AnyDataType; +import org.apache.doris.nereids.types.coercion.FollowToAnyDataType; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -48,16 +41,7 @@ public class ArrayMin extends ScalarFunction implements ExplicitlyCastableSignat UnaryExpression, AlwaysNullable { public static final List SIGNATURES = ImmutableList.of( - FunctionSignature.ret(BooleanType.INSTANCE).args(ArrayType.of(BooleanType.INSTANCE)), - FunctionSignature.ret(TinyIntType.INSTANCE).args(ArrayType.of(TinyIntType.INSTANCE)), - FunctionSignature.ret(SmallIntType.INSTANCE).args(ArrayType.of(SmallIntType.INSTANCE)), - FunctionSignature.ret(IntegerType.INSTANCE).args(ArrayType.of(IntegerType.INSTANCE)), - FunctionSignature.ret(BigIntType.INSTANCE).args(ArrayType.of(BigIntType.INSTANCE)), - FunctionSignature.ret(LargeIntType.INSTANCE).args(ArrayType.of(LargeIntType.INSTANCE)), - FunctionSignature.ret(DecimalV3Type.WILDCARD).args(ArrayType.of(DecimalV3Type.WILDCARD)), - FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(ArrayType.of(DecimalV2Type.SYSTEM_DEFAULT)), - FunctionSignature.ret(FloatType.INSTANCE).args(ArrayType.of(FloatType.INSTANCE)), - FunctionSignature.ret(DoubleType.INSTANCE).args(ArrayType.of(DoubleType.INSTANCE)) + FunctionSignature.ret(new FollowToAnyDataType(0)).args(ArrayType.of(new AnyDataType(0))) ); /** @@ -67,6 +51,14 @@ public ArrayMin(Expression arg) { super("array_min", arg); } + @Override + public void checkLegalityBeforeTypeCoercion() { + DataType argType = child().getDataType(); + if (((ArrayType) argType).getItemType().isComplexType()) { + throw new AnalysisException("array_min does not support complex types: " + toSql()); + } + } + @Override public DataType getDataType() { return ((ArrayType) (child().getDataType())).getItemType(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayReverseSort.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayReverseSort.java index bcdc1d852f57ed..dd62fdb7e453f1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayReverseSort.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayReverseSort.java @@ -18,12 +18,14 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable; 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; +import org.apache.doris.nereids.types.DataType; import org.apache.doris.nereids.types.coercion.AnyDataType; import com.google.common.base.Preconditions; @@ -48,6 +50,14 @@ public ArrayReverseSort(Expression arg) { super("array_reverse_sort", arg); } + @Override + public void checkLegalityBeforeTypeCoercion() { + DataType argType = child().getDataType(); + if (((ArrayType) argType).getItemType().isComplexType()) { + throw new AnalysisException("array_reverse_sort does not support complex types: " + toSql()); + } + } + /** * withChildren. */ diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java index 5953d69b66896b..80f359b61f5d22 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java @@ -18,12 +18,14 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable; 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; +import org.apache.doris.nereids.types.DataType; import org.apache.doris.nereids.types.coercion.AnyDataType; import com.google.common.base.Preconditions; @@ -48,6 +50,14 @@ public ArraySort(Expression arg) { super("array_sort", arg); } + @Override + public void checkLegalityBeforeTypeCoercion() { + DataType argType = child().getDataType(); + if (((ArrayType) argType).getItemType().isComplexType()) { + throw new AnalysisException("array_sort does not support complex types: " + toSql()); + } + } + /** * withChildren. */ From 3db78c2ad0fd2b773560625d9dae5817e8a75134 Mon Sep 17 00:00:00 2001 From: amorynan Date: Fri, 16 Aug 2024 16:28:56 +0800 Subject: [PATCH 3/3] fix cases --- .../suites/nereids_function_p0/scalar_function/Array.groovy | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy b/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy index 1c10896e9f1f35..9fa1af3d635a8d 100644 --- a/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy +++ b/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy @@ -1309,7 +1309,7 @@ suite("nereids_scalar_fn_Array") { } } - // with array empty + sql """ set enable_fold_constant_by_be=true; """ qt_array_empty_fe """select array()""" // array_map with string is can be succeed @@ -1321,9 +1321,6 @@ suite("nereids_scalar_fn_Array") { exception("errCode = 2") } - sql """ set enable_fold_constant_by_be=true; """ - qt_array_empty_be """select array()""" - // array_min/max with nested array for args test { sql "select array_min(array(1,2,3),array(4,5,6));"