diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/GroupConcat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/GroupConcat.java index 61cd525e65117b..e34fba23048204 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/GroupConcat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/GroupConcat.java @@ -23,7 +23,6 @@ import org.apache.doris.nereids.trees.expressions.OrderExpression; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; -import org.apache.doris.nereids.types.DataType; import org.apache.doris.nereids.types.VarcharType; import org.apache.doris.nereids.types.coercion.AnyDataType; import org.apache.doris.nereids.util.ExpressionUtils; @@ -39,10 +38,18 @@ public class GroupConcat extends NullableAggregateFunction implements ExplicitlyCastableSignature, SupportMultiDistinct { - public static final List SIGNATURES = ImmutableList.of( - FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT), + private static final List ONE_ARG = ImmutableList.of( + FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT) + ); + private static final List ONE_ARG_WITH_ORDER_BY = ImmutableList.of( FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) - .varArgs(VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX), + .varArgs(VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX) + ); + private static final List TWO_ARGS = ImmutableList.of( + FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) + .args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT) + ); + private static final List TWO_ARGS_WITH_ORDER_BY = ImmutableList.of( FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) .varArgs(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX) ); @@ -93,23 +100,6 @@ public List getDistinctArguments() { } } - @Override - public void checkLegalityBeforeTypeCoercion() { - DataType typeOrArg0 = getArgumentType(0); - if (!typeOrArg0.isStringLikeType() && !typeOrArg0.isNullType()) { - throw new AnalysisException( - "group_concat requires first parameter to be of type STRING: " + this.toSql()); - } - - if (nonOrderArguments == 2) { - DataType typeOrArg1 = getArgumentType(1); - if (!typeOrArg1.isStringLikeType() && !typeOrArg1.isNullType()) { - throw new AnalysisException( - "group_concat requires second parameter to be of type STRING: " + this.toSql()); - } - } - } - @Override public GroupConcat withAlwaysNullable(boolean alwaysNullable) { return new GroupConcat(distinct, alwaysNullable, children); @@ -130,7 +120,17 @@ public R accept(ExpressionVisitor visitor, C context) { @Override public List getSignatures() { - return SIGNATURES; + if (nonOrderArguments == 2) { + if (arity() >= 3) { + return TWO_ARGS_WITH_ORDER_BY; + } + return TWO_ARGS; + } else { + if (arity() >= 2) { + return ONE_ARG_WITH_ORDER_BY; + } + return ONE_ARG; + } } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/MultiDistinctGroupConcat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/MultiDistinctGroupConcat.java index 30bce48d67c7ee..8c9665fee40d1a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/MultiDistinctGroupConcat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/MultiDistinctGroupConcat.java @@ -23,8 +23,6 @@ import org.apache.doris.nereids.trees.expressions.OrderExpression; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; -import org.apache.doris.nereids.types.CharType; -import org.apache.doris.nereids.types.StringType; import org.apache.doris.nereids.types.VarcharType; import org.apache.doris.nereids.types.coercion.AnyDataType; import org.apache.doris.nereids.util.ExpressionUtils; @@ -37,26 +35,24 @@ /** MultiDistinctGroupConcat */ public class MultiDistinctGroupConcat extends NullableAggregateFunction implements ExplicitlyCastableSignature, MultiDistinction { - public static final List SIGNATURES = ImmutableList.of( - FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT), - FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).varArgs(VarcharType.SYSTEM_DEFAULT, - AnyDataType.INSTANCE_WITHOUT_INDEX), - FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).varArgs(VarcharType.SYSTEM_DEFAULT, - VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX), - - FunctionSignature.ret(StringType.INSTANCE).args(StringType.INSTANCE), - FunctionSignature.ret(StringType.INSTANCE).varArgs(StringType.INSTANCE, - AnyDataType.INSTANCE_WITHOUT_INDEX), - FunctionSignature.ret(StringType.INSTANCE).varArgs(StringType.INSTANCE, - StringType.INSTANCE, AnyDataType.INSTANCE_WITHOUT_INDEX), - - FunctionSignature.ret(CharType.SYSTEM_DEFAULT).args(CharType.SYSTEM_DEFAULT), - FunctionSignature.ret(CharType.SYSTEM_DEFAULT).varArgs(CharType.SYSTEM_DEFAULT, - AnyDataType.INSTANCE_WITHOUT_INDEX), - FunctionSignature.ret(CharType.SYSTEM_DEFAULT).varArgs(CharType.SYSTEM_DEFAULT, - CharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX)); + private static final List ONE_ARG = ImmutableList.of( + FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT) + ); + private static final List ONE_ARG_WITH_ORDER_BY = ImmutableList.of( + FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) + .varArgs(VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX) + ); + private static final List TWO_ARGS = ImmutableList.of( + FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) + .args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT) + ); + private static final List TWO_ARGS_WITH_ORDER_BY = ImmutableList.of( + FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) + .varArgs(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX) + ); private final boolean mustUseMultiDistinctAgg; + private final int nonOrderArguments; /** * constructor with 1 argument with other arguments. @@ -79,8 +75,8 @@ private MultiDistinctGroupConcat(boolean alwaysNullable, Expression arg, private MultiDistinctGroupConcat(boolean mustUseMultiDistinctAgg, boolean alwaysNullable, List args) { super("multi_distinct_group_concat", false, alwaysNullable, args); - checkArguments(children); this.mustUseMultiDistinctAgg = mustUseMultiDistinctAgg; + this.nonOrderArguments = findOrderExprIndex(children); } @Override @@ -99,7 +95,6 @@ public MultiDistinctGroupConcat withAlwaysNullable(boolean alwaysNullable) { */ @Override public MultiDistinctGroupConcat withDistinctAndChildren(boolean distinct, List children) { - checkArguments(children); return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg, alwaysNullable, children); } @@ -110,10 +105,30 @@ public R accept(ExpressionVisitor visitor, C context) { @Override public List getSignatures() { - return SIGNATURES; + if (nonOrderArguments == 2) { + if (arity() >= 3) { + return TWO_ARGS_WITH_ORDER_BY; + } + return TWO_ARGS; + } else { + if (arity() >= 2) { + return ONE_ARG_WITH_ORDER_BY; + } + return ONE_ARG; + } + } + + @Override + public boolean mustUseMultiDistinctAgg() { + return mustUseMultiDistinctAgg || children.stream().anyMatch(OrderExpression.class::isInstance); } - private void checkArguments(List children) { + @Override + public Expression withMustUseMultiDistinctAgg(boolean mustUseMultiDistinctAgg) { + return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg, alwaysNullable, children); + } + + private int findOrderExprIndex(List children) { Preconditions.checkArgument(children().size() >= 1, "children's size should >= 1"); boolean foundOrderExpr = false; int firstOrderExrIndex = 0; @@ -133,15 +148,6 @@ private void checkArguments(List children) { throw new AnalysisException( "multi_distinct_group_concat requires one or two parameters: " + children); } - } - - @Override - public boolean mustUseMultiDistinctAgg() { - return mustUseMultiDistinctAgg || children.stream().anyMatch(OrderExpression.class::isInstance); - } - - @Override - public Expression withMustUseMultiDistinctAgg(boolean mustUseMultiDistinctAgg) { - return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg, alwaysNullable, children); + return firstOrderExrIndex; } } diff --git a/regression-test/data/query_p0/group_concat/test_group_concat.out b/regression-test/data/query_p0/group_concat/test_group_concat.out index 3065713cf55c49..29afb6c950d88d 100644 --- a/regression-test/data/query_p0/group_concat/test_group_concat.out +++ b/regression-test/data/query_p0/group_concat/test_group_concat.out @@ -39,6 +39,10 @@ false 1 2 1 2 +-- !select_12 -- +1 2 +1 2 + -- !select_13 -- 1 2 1 2 @@ -68,7 +72,7 @@ false 2 23,222,22,211,21 -- !select_group_concat_order_by1 -- -1,11,2,21,21,211,22,222,23,3 3,23,222,22,211,21,21,2,11,1 +1,11,2,21,21,211,22,222,23,3 3223222222222112212212221121 -- !select_group_concat_order_by2 -- 1,11,2,21,21,211,22,222,23,3 3,23,222,22,211,21,21,2,11,1 diff --git a/regression-test/suites/query_p0/group_concat/test_group_concat.groovy b/regression-test/suites/query_p0/group_concat/test_group_concat.groovy index ee9bab29e8e166..a60514c719f687 100644 --- a/regression-test/suites/query_p0/group_concat/test_group_concat.groovy +++ b/regression-test/suites/query_p0/group_concat/test_group_concat.groovy @@ -72,7 +72,16 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") { qt_select_12 """ select - group_concat( distinct b1, '?'), group_concat( distinct b3, '?') + group_concat( distinct b1, 123), group_concat( distinct b3, '?') + from + table_group_concat + group by + b2; + """ + + qt_select_12 """ + select + multi_distinct_group_concat(b1, 123), multi_distinct_group_concat(b3, '?') from table_group_concat group by @@ -109,7 +118,7 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") { select * from table_group_concat order by b1, b2, b3; """ qt_select_group_concat_order_by_desc1 """ - SELECT b1, group_concat(cast(abs(b2) as varchar) order by abs(b2) desc) FROM table_group_concat group by b1 order by b1 + SELECT b1, group_concat(abs(b2) order by abs(b2) desc) FROM table_group_concat group by b1 order by b1 """ qt_select_group_concat_order_by_desc2 """ @@ -119,12 +128,13 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") { SELECT b1, group_concat(cast(abs(b3) as varchar) order by abs(b2) desc, b3 desc) FROM table_group_concat group by b1 order by b1 """ qt_select_group_concat_order_by1 """ - select group_concat(b3,',' order by b3 asc),group_concat(b3,',' order by b3 desc) from table_group_concat; + select group_concat(b3,',' order by b3 asc),group_concat(b3,'2' order by b3 desc) from table_group_concat; """ sql """create view if not exists test_view as select group_concat(b3,',' order by b3 asc),group_concat(b3,',' order by b3 desc) from table_group_concat;""" qt_select_group_concat_order_by2 """ select * from test_view; """ + sql """drop view if exists test_view""" }