From a34020115d3e457c0e64f85d9293caab8784d462 Mon Sep 17 00:00:00 2001 From: amorynan Date: Tue, 13 Aug 2024 18:00:49 +0800 Subject: [PATCH 1/2] 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 | 31 ++++++++++++++++++ .../test_array_functions_by_literal.groovy | 32 +++++++++++++++++++ 5 files changed, 110 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 ef3813d6deb20c..e4e5b6eb7a2173 100644 --- a/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy +++ b/regression-test/suites/nereids_function_p0/scalar_function/Array.groovy @@ -1310,4 +1310,35 @@ suite("nereids_scalar_fn_Array") { qt_array_empty_fe """select array()""" sql """ set debug_skip_fold_constant=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 f335cd72115cb4..fd372dfbd038bd 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 @@ -410,4 +410,36 @@ suite("test_array_functions_by_literal") { } catch (Exception ex) { assert("${ex}".contains("errCode = 2, detailMessage = No matching function with signature: array_intersect")) } + + // 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 e62c415fac1f4a9ad9e7791224dbd5d87731cc6e Mon Sep 17 00:00:00 2001 From: amorynan Date: Tue, 13 Aug 2024 19:59:28 +0800 Subject: [PATCH 2/2] 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 38833ecc2b44f2..1fb920e0bd1d4c 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. */