Skip to content

Commit 690f199

Browse files
github-actions[bot]morrySnow
github-actions[bot]
and
morrySnow
authored
branch-3.1: [opt](group_concat) allow args be types other than string #52805 (#53012)
Cherry-picked from #52805 Co-authored-by: morrySnow <zhangwenxin@selectdb.com>
1 parent 09a158d commit 690f199

4 files changed

Lines changed: 80 additions & 60 deletions

File tree

‎fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/GroupConcat.java‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.apache.doris.nereids.trees.expressions.OrderExpression;
2424
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
2525
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
26-
import org.apache.doris.nereids.types.DataType;
2726
import org.apache.doris.nereids.types.VarcharType;
2827
import org.apache.doris.nereids.types.coercion.AnyDataType;
2928
import org.apache.doris.nereids.util.ExpressionUtils;
@@ -39,10 +38,18 @@
3938
public class GroupConcat extends NullableAggregateFunction
4039
implements ExplicitlyCastableSignature, SupportMultiDistinct {
4140

42-
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
43-
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT),
41+
private static final List<FunctionSignature> ONE_ARG = ImmutableList.of(
42+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT)
43+
);
44+
private static final List<FunctionSignature> ONE_ARG_WITH_ORDER_BY = ImmutableList.of(
4445
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
45-
.varArgs(VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX),
46+
.varArgs(VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX)
47+
);
48+
private static final List<FunctionSignature> TWO_ARGS = ImmutableList.of(
49+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
50+
.args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT)
51+
);
52+
private static final List<FunctionSignature> TWO_ARGS_WITH_ORDER_BY = ImmutableList.of(
4653
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
4754
.varArgs(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX)
4855
);
@@ -93,23 +100,6 @@ public List<Expression> getDistinctArguments() {
93100
}
94101
}
95102

96-
@Override
97-
public void checkLegalityBeforeTypeCoercion() {
98-
DataType typeOrArg0 = getArgumentType(0);
99-
if (!typeOrArg0.isStringLikeType() && !typeOrArg0.isNullType()) {
100-
throw new AnalysisException(
101-
"group_concat requires first parameter to be of type STRING: " + this.toSql());
102-
}
103-
104-
if (nonOrderArguments == 2) {
105-
DataType typeOrArg1 = getArgumentType(1);
106-
if (!typeOrArg1.isStringLikeType() && !typeOrArg1.isNullType()) {
107-
throw new AnalysisException(
108-
"group_concat requires second parameter to be of type STRING: " + this.toSql());
109-
}
110-
}
111-
}
112-
113103
@Override
114104
public GroupConcat withAlwaysNullable(boolean alwaysNullable) {
115105
return new GroupConcat(distinct, alwaysNullable, children);
@@ -130,7 +120,17 @@ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
130120

131121
@Override
132122
public List<FunctionSignature> getSignatures() {
133-
return SIGNATURES;
123+
if (nonOrderArguments == 2) {
124+
if (arity() >= 3) {
125+
return TWO_ARGS_WITH_ORDER_BY;
126+
}
127+
return TWO_ARGS;
128+
} else {
129+
if (arity() >= 2) {
130+
return ONE_ARG_WITH_ORDER_BY;
131+
}
132+
return ONE_ARG;
133+
}
134134
}
135135

136136
@Override

‎fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/MultiDistinctGroupConcat.java‎

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import org.apache.doris.nereids.trees.expressions.OrderExpression;
2424
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
2525
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
26-
import org.apache.doris.nereids.types.CharType;
27-
import org.apache.doris.nereids.types.StringType;
2826
import org.apache.doris.nereids.types.VarcharType;
2927
import org.apache.doris.nereids.types.coercion.AnyDataType;
3028
import org.apache.doris.nereids.util.ExpressionUtils;
@@ -37,26 +35,24 @@
3735
/** MultiDistinctGroupConcat */
3836
public class MultiDistinctGroupConcat extends NullableAggregateFunction
3937
implements ExplicitlyCastableSignature, MultiDistinction {
40-
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
41-
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT),
42-
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).varArgs(VarcharType.SYSTEM_DEFAULT,
43-
AnyDataType.INSTANCE_WITHOUT_INDEX),
44-
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).varArgs(VarcharType.SYSTEM_DEFAULT,
45-
VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX),
46-
47-
FunctionSignature.ret(StringType.INSTANCE).args(StringType.INSTANCE),
48-
FunctionSignature.ret(StringType.INSTANCE).varArgs(StringType.INSTANCE,
49-
AnyDataType.INSTANCE_WITHOUT_INDEX),
50-
FunctionSignature.ret(StringType.INSTANCE).varArgs(StringType.INSTANCE,
51-
StringType.INSTANCE, AnyDataType.INSTANCE_WITHOUT_INDEX),
52-
53-
FunctionSignature.ret(CharType.SYSTEM_DEFAULT).args(CharType.SYSTEM_DEFAULT),
54-
FunctionSignature.ret(CharType.SYSTEM_DEFAULT).varArgs(CharType.SYSTEM_DEFAULT,
55-
AnyDataType.INSTANCE_WITHOUT_INDEX),
56-
FunctionSignature.ret(CharType.SYSTEM_DEFAULT).varArgs(CharType.SYSTEM_DEFAULT,
57-
CharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX));
38+
private static final List<FunctionSignature> ONE_ARG = ImmutableList.of(
39+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT).args(VarcharType.SYSTEM_DEFAULT)
40+
);
41+
private static final List<FunctionSignature> ONE_ARG_WITH_ORDER_BY = ImmutableList.of(
42+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
43+
.varArgs(VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX)
44+
);
45+
private static final List<FunctionSignature> TWO_ARGS = ImmutableList.of(
46+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
47+
.args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT)
48+
);
49+
private static final List<FunctionSignature> TWO_ARGS_WITH_ORDER_BY = ImmutableList.of(
50+
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
51+
.varArgs(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT, AnyDataType.INSTANCE_WITHOUT_INDEX)
52+
);
5853

5954
private final boolean mustUseMultiDistinctAgg;
55+
private final int nonOrderArguments;
6056

6157
/**
6258
* constructor with 1 argument with other arguments.
@@ -79,8 +75,8 @@ private MultiDistinctGroupConcat(boolean alwaysNullable, Expression arg,
7975

8076
private MultiDistinctGroupConcat(boolean mustUseMultiDistinctAgg, boolean alwaysNullable, List<Expression> args) {
8177
super("multi_distinct_group_concat", false, alwaysNullable, args);
82-
checkArguments(children);
8378
this.mustUseMultiDistinctAgg = mustUseMultiDistinctAgg;
79+
this.nonOrderArguments = findOrderExprIndex(children);
8480
}
8581

8682
@Override
@@ -99,7 +95,6 @@ public MultiDistinctGroupConcat withAlwaysNullable(boolean alwaysNullable) {
9995
*/
10096
@Override
10197
public MultiDistinctGroupConcat withDistinctAndChildren(boolean distinct, List<Expression> children) {
102-
checkArguments(children);
10398
return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg, alwaysNullable, children);
10499
}
105100

@@ -110,10 +105,30 @@ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
110105

111106
@Override
112107
public List<FunctionSignature> getSignatures() {
113-
return SIGNATURES;
108+
if (nonOrderArguments == 2) {
109+
if (arity() >= 3) {
110+
return TWO_ARGS_WITH_ORDER_BY;
111+
}
112+
return TWO_ARGS;
113+
} else {
114+
if (arity() >= 2) {
115+
return ONE_ARG_WITH_ORDER_BY;
116+
}
117+
return ONE_ARG;
118+
}
119+
}
120+
121+
@Override
122+
public boolean mustUseMultiDistinctAgg() {
123+
return mustUseMultiDistinctAgg || children.stream().anyMatch(OrderExpression.class::isInstance);
114124
}
115125

116-
private void checkArguments(List<Expression> children) {
126+
@Override
127+
public Expression withMustUseMultiDistinctAgg(boolean mustUseMultiDistinctAgg) {
128+
return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg, alwaysNullable, children);
129+
}
130+
131+
private int findOrderExprIndex(List<Expression> children) {
117132
Preconditions.checkArgument(children().size() >= 1, "children's size should >= 1");
118133
boolean foundOrderExpr = false;
119134
int firstOrderExrIndex = 0;
@@ -133,15 +148,6 @@ private void checkArguments(List<Expression> children) {
133148
throw new AnalysisException(
134149
"multi_distinct_group_concat requires one or two parameters: " + children);
135150
}
136-
}
137-
138-
@Override
139-
public boolean mustUseMultiDistinctAgg() {
140-
return mustUseMultiDistinctAgg || children.stream().anyMatch(OrderExpression.class::isInstance);
141-
}
142-
143-
@Override
144-
public Expression withMustUseMultiDistinctAgg(boolean mustUseMultiDistinctAgg) {
145-
return new MultiDistinctGroupConcat(mustUseMultiDistinctAgg, alwaysNullable, children);
151+
return firstOrderExrIndex;
146152
}
147153
}

‎regression-test/data/query_p0/group_concat/test_group_concat.out‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ false
3939
1 2
4040
1 2
4141

42+
-- !select_12 --
43+
1 2
44+
1 2
45+
4246
-- !select_13 --
4347
1 2
4448
1 2
@@ -68,7 +72,7 @@ false
6872
2 23,222,22,211,21
6973

7074
-- !select_group_concat_order_by1 --
71-
1,11,2,21,21,211,22,222,23,3 3,23,222,22,211,21,21,2,11,1
75+
1,11,2,21,21,211,22,222,23,3 3223222222222112212212221121
7276

7377
-- !select_group_concat_order_by2 --
7478
1,11,2,21,21,211,22,222,23,3 3,23,222,22,211,21,21,2,11,1

‎regression-test/suites/query_p0/group_concat/test_group_concat.groovy‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,16 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") {
7272

7373
qt_select_12 """
7474
select
75-
group_concat( distinct b1, '?'), group_concat( distinct b3, '?')
75+
group_concat( distinct b1, 123), group_concat( distinct b3, '?')
76+
from
77+
table_group_concat
78+
group by
79+
b2;
80+
"""
81+
82+
qt_select_12 """
83+
select
84+
multi_distinct_group_concat(b1, 123), multi_distinct_group_concat(b3, '?')
7685
from
7786
table_group_concat
7887
group by
@@ -109,7 +118,7 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") {
109118
select * from table_group_concat order by b1, b2, b3;
110119
"""
111120
qt_select_group_concat_order_by_desc1 """
112-
SELECT b1, group_concat(cast(abs(b2) as varchar) order by abs(b2) desc) FROM table_group_concat group by b1 order by b1
121+
SELECT b1, group_concat(abs(b2) order by abs(b2) desc) FROM table_group_concat group by b1 order by b1
113122
"""
114123

115124
qt_select_group_concat_order_by_desc2 """
@@ -119,12 +128,13 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") {
119128
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
120129
"""
121130
qt_select_group_concat_order_by1 """
122-
select group_concat(b3,',' order by b3 asc),group_concat(b3,',' order by b3 desc) from table_group_concat;
131+
select group_concat(b3,',' order by b3 asc),group_concat(b3,'2' order by b3 desc) from table_group_concat;
123132
"""
124133

125134
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;"""
126135
qt_select_group_concat_order_by2 """
127136
select * from test_view;
128137
"""
138+
129139
sql """drop view if exists test_view"""
130140
}

0 commit comments

Comments
 (0)