This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.1 by this push:
new 690f19909d5 branch-3.1: [opt](group_concat) allow args be types other
than string #52805 (#53012)
690f19909d5 is described below
commit 690f19909d583706d96755bc57896d5780203ab8
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Jul 10 10:06:22 2025 +0800
branch-3.1: [opt](group_concat) allow args be types other than string
#52805 (#53012)
Cherry-picked from #52805
Co-authored-by: morrySnow <[email protected]>
---
.../expressions/functions/agg/GroupConcat.java | 44 ++++++------
.../functions/agg/MultiDistinctGroupConcat.java | 74 +++++++++++----------
.../query_p0/group_concat/test_group_concat.out | Bin 1158 -> 1184 bytes
.../query_p0/group_concat/test_group_concat.groovy | 16 ++++-
4 files changed, 75 insertions(+), 59 deletions(-)
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 61cd525e651..e34fba23048 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.Expression;
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 @@ import java.util.List;
public class GroupConcat extends NullableAggregateFunction
implements ExplicitlyCastableSignature, SupportMultiDistinct {
- public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
-
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT),
+ private static final List<FunctionSignature> ONE_ARG = ImmutableList.of(
+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT)
+ );
+ private static final List<FunctionSignature> 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<FunctionSignature> TWO_ARGS = ImmutableList.of(
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .args(VarcharType.SYSTEM_DEFAULT,
VarcharType.SYSTEM_DEFAULT)
+ );
+ private static final List<FunctionSignature> 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 class GroupConcat extends NullableAggregateFunction
}
}
- @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 class GroupConcat extends NullableAggregateFunction
@Override
public List<FunctionSignature> 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 30bce48d67c..8c9665fee40 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.Expression;
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 @@ import java.util.List;
/** MultiDistinctGroupConcat */
public class MultiDistinctGroupConcat extends NullableAggregateFunction
implements ExplicitlyCastableSignature, MultiDistinction {
- public static final List<FunctionSignature> 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<FunctionSignature> ONE_ARG = ImmutableList.of(
+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT)
+ );
+ private static final List<FunctionSignature> ONE_ARG_WITH_ORDER_BY =
ImmutableList.of(
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .varArgs(VarcharType.SYSTEM_DEFAULT,
AnyDataType.INSTANCE_WITHOUT_INDEX)
+ );
+ private static final List<FunctionSignature> TWO_ARGS = ImmutableList.of(
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .args(VarcharType.SYSTEM_DEFAULT,
VarcharType.SYSTEM_DEFAULT)
+ );
+ private static final List<FunctionSignature> 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 @@ public class MultiDistinctGroupConcat extends
NullableAggregateFunction
private MultiDistinctGroupConcat(boolean mustUseMultiDistinctAgg, boolean
alwaysNullable, List<Expression> args) {
super("multi_distinct_group_concat", false, alwaysNullable, args);
- checkArguments(children);
this.mustUseMultiDistinctAgg = mustUseMultiDistinctAgg;
+ this.nonOrderArguments = findOrderExprIndex(children);
}
@Override
@@ -99,7 +95,6 @@ public class MultiDistinctGroupConcat extends
NullableAggregateFunction
*/
@Override
public MultiDistinctGroupConcat withDistinctAndChildren(boolean distinct,
List<Expression> children) {
- checkArguments(children);
return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg,
alwaysNullable, children);
}
@@ -110,10 +105,30 @@ public class MultiDistinctGroupConcat extends
NullableAggregateFunction
@Override
public List<FunctionSignature> 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<Expression> children) {
+ @Override
+ public Expression withMustUseMultiDistinctAgg(boolean
mustUseMultiDistinctAgg) {
+ return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg,
alwaysNullable, children);
+ }
+
+ private int findOrderExprIndex(List<Expression> children) {
Preconditions.checkArgument(children().size() >= 1, "children's size
should >= 1");
boolean foundOrderExpr = false;
int firstOrderExrIndex = 0;
@@ -133,15 +148,6 @@ public class MultiDistinctGroupConcat extends
NullableAggregateFunction
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 3065713cf55..29afb6c950d 100644
Binary files a/regression-test/data/query_p0/group_concat/test_group_concat.out
and b/regression-test/data/query_p0/group_concat/test_group_concat.out differ
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 ee9bab29e8e..a60514c719f 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"""
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
