diff --git a/be/src/exec/aggregator.cpp b/be/src/exec/aggregator.cpp index 5235cf9650d21..8bfbf394b3c10 100644 --- a/be/src/exec/aggregator.cpp +++ b/be/src/exec/aggregator.cpp @@ -178,7 +178,8 @@ void AggregatorParams::init() { agg_fn_types[i] = {return_type, serde_type, arg_typedescs, has_nullable_child, is_nullable}; agg_fn_types[i].is_always_nullable_result = ALWAYS_NULLABLE_RESULT_AGG_FUNCS.contains(fn.name.function_name); - if (fn.name.function_name == "array_agg" || fn.name.function_name == "group_concat") { + // TODO(fixme): move this to FE + if (fn.name.function_name == "array_agg" || fn.name.function_name == "group_concat2") { // set order by info if (fn.aggregate_fn.__isset.is_asc_order && fn.aggregate_fn.__isset.nulls_first && !fn.aggregate_fn.is_asc_order.empty()) { @@ -470,8 +471,12 @@ Status Aggregator::prepare(RuntimeState* state, ObjectPool* pool, RuntimeProfile _agg_fn_ctxs[i] = FunctionContext::create_context(state, _mem_pool.get(), return_type, arg_types, agg_fn_type.is_distinct, agg_fn_type.is_asc_order, agg_fn_type.nulls_first); + auto& ctx_query_options = _agg_fn_ctxs[i]->get_ctx_query_options(); if (state->query_options().__isset.group_concat_max_len) { - _agg_fn_ctxs[i]->set_group_concat_max_len(state->query_options().group_concat_max_len); + ctx_query_options.set_group_concat_max_len(state->query_options().group_concat_max_len); + } + if (state->query_options().__isset.default_group_concat_separator) { + ctx_query_options.set_default_group_concat_separator(state->query_options().default_group_concat_separator); } state->obj_pool()->add(_agg_fn_ctxs[i]); _agg_fn_ctxs[i]->set_mem_usage_counter(&_agg_state_mem_usage); @@ -512,7 +517,7 @@ Status Aggregator::_create_aggregate_function(starrocks::RuntimeState* state, co } // check whether it's _merge/_union combinator if it contains agg state type - auto& func_name = fn.name.function_name; + auto func_name = fn.name.function_name; if (fn.__isset.agg_state_desc) { if (arg_types.size() != 1) { return Status::InternalError(strings::Substitute("Invalid agg function plan: $0 with (arg type $1)", @@ -559,6 +564,11 @@ Status Aggregator::_create_aggregate_function(starrocks::RuntimeState* state, co TypeDescriptor serde_type = TypeDescriptor::from_thrift(fn.aggregate_fn.intermediate_type); DCHECK_LE(1, fn.arg_types.size()); TypeDescriptor arg_type = arg_types[0]; + + // To be compatible with old versions, change group_concat2 name to group_concat if the intermediate type is string. + if (fn.name.function_name == "group_concat" && serde_type.type == TYPE_STRUCT) { + func_name = "group_concat2"; + } auto* func = get_aggregate_function(func_name, return_type, arg_types, is_result_nullable, fn.binary_type, state->func_version()); if (func == nullptr) { diff --git a/be/src/exprs/agg/factory/aggregate_factory.cpp b/be/src/exprs/agg/factory/aggregate_factory.cpp index d790204ba91eb..59ab010951a26 100644 --- a/be/src/exprs/agg/factory/aggregate_factory.cpp +++ b/be/src/exprs/agg/factory/aggregate_factory.cpp @@ -131,12 +131,6 @@ static const AggregateFunction* get_function(const std::string& name, LogicalTyp } } - if (func_version > 6) { - if (name == "group_concat") { - func_name = "group_concat2"; - } - } - if (binary_type == TFunctionBinaryType::BUILTIN) { auto func = AggregateFuncResolver::instance()->get_aggregate_info(func_name, arg_type, return_type, is_window_function, is_null); diff --git a/be/src/exprs/agg/group_concat.h b/be/src/exprs/agg/group_concat.h index 04b620965dc0a..493e030e5d14e 100644 --- a/be/src/exprs/agg/group_concat.h +++ b/be/src/exprs/agg/group_concat.h @@ -102,17 +102,18 @@ class GroupConcatAggregateFunction std::string& result = this->data(state).intermediate_string; Slice val = column_val->get_slice(row_num); + auto separator = ctx->get_ctx_query_options().default_group_concat_separator; //DEFAULT sep_length. if (!this->data(state).initial) { this->data(state).initial = true; // separator's length; - uint32_t size = 2; + uint32_t size = separator.size(); result.append(reinterpret_cast(&size), sizeof(uint32_t)) - .append(", ") + .append(separator) .append(val.get_data(), val.get_size()); } else { - result.append(", ").append(val.get_data(), val.get_size()); + result.append(separator).append(val.get_data(), val.get_size()); } } } @@ -258,8 +259,9 @@ class GroupConcatAggregateFunction const auto* column_value = down_cast(src[0].get()); if (chunk_size > 0) { - const char* sep = ", "; - const uint32_t size_sep = 2; + auto separator = ctx->get_ctx_query_options().default_group_concat_separator; + auto sep = separator.data(); + const uint32_t size_sep = separator.size(); size_t old_size = bytes.size(); CHECK_EQ(old_size, 0); @@ -666,7 +668,7 @@ class GroupConcatAggregateFunctionV2 bytes.resize(offset + length); bool overflow = false; - size_t limit = ctx->get_group_concat_max_len() + offset; + size_t limit = ctx->get_ctx_query_options().group_concat_max_len + offset; auto last_unique_row_id = elem_size - 1; for (auto i = elem_size - 1; i >= 0; i--) { auto idx = i; diff --git a/be/src/exprs/function_context.h b/be/src/exprs/function_context.h index f781ddc0aa984..66c1bc977e628 100644 --- a/be/src/exprs/function_context.h +++ b/be/src/exprs/function_context.h @@ -63,6 +63,19 @@ class FunctionContext { THREAD_LOCAL, }; + // Query options for query runtime. + struct QueryOptions { + // min value is 4, default is 1024 + ssize_t group_concat_max_len = 1024; + std::string default_group_concat_separator = ","; + + public: + void set_group_concat_max_len(ssize_t len) { this->group_concat_max_len = len < 4 ? 4 : len; } + void set_default_group_concat_separator(std::string separator) { + this->default_group_concat_separator = separator; + } + }; + /// Create a FunctionContext for a UDF. Caller is responsible for deleting it. static FunctionContext* create_context(RuntimeState* state, MemPool* pool, const FunctionContext::TypeDesc& return_type, @@ -168,9 +181,7 @@ class FunctionContext { void release_mems(); - ssize_t get_group_concat_max_len() { return group_concat_max_len; } - // min value is 4, default is 1024 - void set_group_concat_max_len(ssize_t len) { group_concat_max_len = len < 4 ? 4 : len; } + FunctionContext::QueryOptions& get_ctx_query_options() { return _query_options; } bool error_if_overflow() const; @@ -223,7 +234,7 @@ class FunctionContext { std::vector _is_asc_order; std::vector _nulls_first; bool _is_distinct = false; - ssize_t group_concat_max_len = 1024; + QueryOptions _query_options; // used for ngram bloom filter to speed up some function std::unique_ptr _ngramState; diff --git a/be/test/exprs/agg/aggregate_test.cpp b/be/test/exprs/agg/aggregate_test.cpp index 4d722c78fb269..55749c327a910 100644 --- a/be/test/exprs/agg/aggregate_test.cpp +++ b/be/test/exprs/agg/aggregate_test.cpp @@ -1311,7 +1311,7 @@ TEST_F(AggregateTest, test_bitmap_nullable) { ASSERT_EQ(50, result_data.get_data()[0]); } -TEST_F(AggregateTest, test_group_concat) { +TEST_F(AggregateTest, test_group_concat1) { const AggregateFunction* group_concat_function = get_aggregate_function("group_concat", TYPE_VARCHAR, TYPE_VARCHAR, false); auto state = ManagedAggrState::create(ctx, group_concat_function); @@ -1327,6 +1327,8 @@ TEST_F(AggregateTest, test_group_concat) { const Column* row_column = data_column.get(); // test update + auto& query_options = ctx->get_ctx_query_options(); + query_options.set_default_group_concat_separator(", "); group_concat_function->update_batch_single_state(ctx, data_column->size(), &row_column, state->state()); auto result_column = BinaryColumn::create(); @@ -1335,6 +1337,32 @@ TEST_F(AggregateTest, test_group_concat) { ASSERT_EQ("starrocks0, starrocks1, starrocks2, starrocks3, starrocks4, starrocks5", result_column->get_data()[0]); } +TEST_F(AggregateTest, test_group_concat2) { + const AggregateFunction* group_concat_function = + get_aggregate_function("group_concat", TYPE_VARCHAR, TYPE_VARCHAR, false); + auto state = ManagedAggrState::create(ctx, group_concat_function); + + auto data_column = BinaryColumn::create(); + + for (int i = 0; i < 6; i++) { + std::string val("starrocks"); + val.append(std::to_string(i)); + data_column->append(val); + } + + const Column* row_column = data_column.get(); + + // test update + auto& query_options = ctx->get_ctx_query_options(); + query_options.set_default_group_concat_separator(","); + group_concat_function->update_batch_single_state(ctx, data_column->size(), &row_column, state->state()); + + auto result_column = BinaryColumn::create(); + group_concat_function->finalize_to_column(ctx, state->state(), result_column.get()); + + ASSERT_EQ("starrocks0,starrocks1,starrocks2,starrocks3,starrocks4,starrocks5", result_column->get_data()[0]); +} + TEST_F(AggregateTest, test_group_concat_const_seperator) { std::vector arg_types = { AnyValUtil::column_type_to_type_desc(TypeDescriptor::from_logical_type(TYPE_VARCHAR)), diff --git a/fe/fe-core/src/main/java/com/starrocks/analysis/FunctionParams.java b/fe/fe-core/src/main/java/com/starrocks/analysis/FunctionParams.java index f91d0f4509982..bf35c7c4ebbf0 100644 --- a/fe/fe-core/src/main/java/com/starrocks/analysis/FunctionParams.java +++ b/fe/fe-core/src/main/java/com/starrocks/analysis/FunctionParams.java @@ -38,6 +38,7 @@ import com.google.common.collect.Lists; import com.starrocks.catalog.Function; import com.starrocks.common.io.Writable; +import org.apache.commons.collections.CollectionUtils; import java.io.DataInput; import java.io.DataOutput; @@ -112,7 +113,7 @@ public int getOrderByElemNum() { } public String getOrderByStringToSql() { - if (orderByElements != null && !orderByElements.isEmpty()) { + if (!CollectionUtils.isEmpty(orderByElements)) { StringBuilder sb = new StringBuilder(); sb.append(" ORDER BY ").append(orderByElements.stream().map(OrderByElement::toSql). collect(Collectors.joining(" "))); @@ -123,7 +124,7 @@ public String getOrderByStringToSql() { } public String getOrderByStringToExplain() { - if (orderByElements != null && !orderByElements.isEmpty()) { + if (!CollectionUtils.isEmpty(orderByElements)) { StringBuilder sb = new StringBuilder(); sb.append(" ORDER BY ").append(orderByElements.stream().map(OrderByElement::explain). collect(Collectors.joining(" "))); diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/FunctionSet.java b/fe/fe-core/src/main/java/com/starrocks/catalog/FunctionSet.java index 8d906347c7608..479852acb8ff9 100644 --- a/fe/fe-core/src/main/java/com/starrocks/catalog/FunctionSet.java +++ b/fe/fe-core/src/main/java/com/starrocks/catalog/FunctionSet.java @@ -185,7 +185,12 @@ public class FunctionSet { public static final String CONCAT_WS = "concat_ws"; public static final String ENDS_WITH = "ends_with"; public static final String FIND_IN_SET = "find_in_set"; + // multi version of group_concat: + // group_concat : no distinct or order by arguments + // group_concat2: with distinct or order by arguments public static final String GROUP_CONCAT = "group_concat"; + public static final String GROUP_CONCAT_V2 = "group_concat2"; + public static final String INSTR = "instr"; public static final String LCASE = "lcase"; public static final String LEFT = "left"; @@ -576,6 +581,12 @@ public class FunctionSet { ImmutableSet.builder() .addAll(Type.INTEGER_TYPES) .build(); + + public static final Set GROUP_CONCAT_FUNCS = + ImmutableSortedSet.orderedBy(String.CASE_INSENSITIVE_ORDER) + .add(GROUP_CONCAT) + .add(GROUP_CONCAT_V2) + .build(); /** * Use for vectorized engine, but we can't use vectorized function directly, because we * need to check whether the expression tree can use vectorized function from bottom to @@ -1041,7 +1052,16 @@ private void initAggregateBuiltins() { Lists.newArrayList(Type.ANY_ELEMENT), Type.ANY_ARRAY, Type.ANY_STRUCT, true, true, false, false)); + // group_concat(string) addBuiltin(AggregateFunction.createBuiltin(GROUP_CONCAT, + Lists.newArrayList(Type.VARCHAR), Type.VARCHAR, Type.VARBINARY, + false, false, false)); + // group_concat(string, string) + addBuiltin(AggregateFunction.createBuiltin(GROUP_CONCAT, + Lists.newArrayList(Type.VARCHAR, Type.VARCHAR), Type.VARCHAR, Type.VARBINARY, + false, false, false)); + // group_concat with distinct or order by arguments + addBuiltin(AggregateFunction.createBuiltin(GROUP_CONCAT_V2, Lists.newArrayList(Type.ANY_ELEMENT), Type.VARCHAR, Type.ANY_STRUCT, true, false, false, false)); diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java b/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java index 8d94d1c25e4ff..f5998271093ca 100644 --- a/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java @@ -4272,6 +4272,9 @@ public TQueryOptions toThrift() { tResult.setTransmission_encode_level(transmissionEncodeLevel); tResult.setGroup_concat_max_len(groupConcatMaxLen); + if (SqlModeHelper.check(sqlMode, SqlModeHelper.MODE_GROUP_CONCAT_LEGACY)) { + tResult.setDefault_group_concat_separator(", "); + } tResult.setRpc_http_min_size(rpcHttpMinSize); tResult.setInterleaving_group_size(interleavingGroupSize); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AstToStringBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AstToStringBuilder.java index 46599a3994752..ea05fe433622f 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AstToStringBuilder.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AstToStringBuilder.java @@ -1056,7 +1056,11 @@ public String visitFunctionCall(FunctionCallExpr node, Void context) { sb.append("`" + node.getFnName().getDb() + "`."); } String functionName = node.getFnName().getFunction(); - sb.append(functionName); + if (functionName.equals(FunctionSet.GROUP_CONCAT_V2)) { + sb.append(FunctionSet.GROUP_CONCAT); + } else { + sb.append(functionName); + } sb.append("("); if (fnParams.isStar()) { @@ -1075,9 +1079,9 @@ public String visitFunctionCall(FunctionCallExpr node, Void context) { StringLiteral boundary = (StringLiteral) node.getChild(3); sb.append(", ").append(boundary.getValue()); sb.append(")"); - } else if (functionName.equals(FunctionSet.ARRAY_AGG) || functionName.equals(FunctionSet.GROUP_CONCAT)) { + } else if (functionName.equals(FunctionSet.ARRAY_AGG) || functionName.equals(FunctionSet.GROUP_CONCAT_V2)) { int end = 1; - if (functionName.equals(FunctionSet.GROUP_CONCAT)) { + if (functionName.equals(FunctionSet.GROUP_CONCAT_V2)) { end = fnParams.exprs().size() - fnParams.getOrderByElemNum() - 1; } for (int i = 0; i < end && i < node.getChildren().size(); ++i) { @@ -1087,10 +1091,12 @@ public String visitFunctionCall(FunctionCallExpr node, Void context) { sb.append(visit(node.getChild(i))); } List sortClause = fnParams.getOrderByElements(); - if (sortClause != null) { + if (!CollectionUtils.isEmpty(sortClause)) { sb.append(" ORDER BY ").append(visitAstList(sortClause)); } - if (functionName.equals(FunctionSet.GROUP_CONCAT) && end < node.getChildren().size() && end > 0) { + boolean isGroupConcatV2 = functionName.equals(FunctionSet.GROUP_CONCAT_V2) && + (fnParams.isDistinct() || !CollectionUtils.isEmpty(sortClause)); + if (isGroupConcatV2 && end < node.getChildren().size() && end > 0) { sb.append(" SEPARATOR "); sb.append(visit(node.getChild(end))); } diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/ExpressionAnalyzer.java b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/ExpressionAnalyzer.java index a8054854729a1..ebda9c97d2bf2 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/ExpressionAnalyzer.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/ExpressionAnalyzer.java @@ -1153,13 +1153,38 @@ private void checkFunction(String fnName, FunctionCallExpr node, Type[] argument " can't cast from " + node.getChild(1).getType().toSql() + " to ARRAY"); } break; - case FunctionSet.GROUP_CONCAT: + case FunctionSet.GROUP_CONCAT: { + if (node.getChildren().size() > 2 || node.getChildren().isEmpty()) { + throw new SemanticException( + "group_concat requires one or two parameters: " + node.toSql(), + node.getPos()); + } + if (node.getParams().isDistinct()) { + throw new SemanticException("group_concat does not support DISTINCT", node.getPos()); + } + Expr arg0 = node.getChild(0); + if (!Type.canCastTo(arg0.getType(), Type.VARCHAR)) { + throw new SemanticException( + "group_concat requires first parameter to be of getType() STRING: " + node.toSql(), + arg0.getPos()); + } + if (node.getChildren().size() == 2) { + Expr arg1 = node.getChild(1); + if (!Type.canCastTo(arg1.getType(), Type.VARCHAR)) { + throw new SemanticException( + "group_concat requires second parameter to be of getType() STRING: " + + node.toSql(), arg1.getPos()); + } + } + break; + } + case FunctionSet.GROUP_CONCAT_V2: case FunctionSet.ARRAY_AGG: { if (node.getChildren().size() == 0) { throw new SemanticException(fnName + " should have at least one input", node.getPos()); } int start = argumentTypes.length - node.getParams().getOrderByElemNum(); - if (fnName.equals(FunctionSet.GROUP_CONCAT)) { + if (fnName.equals(FunctionSet.GROUP_CONCAT_V2)) { if (start < 2) { throw new SemanticException(fnName + " should have output expressions before [ORDER BY]", node.getPos()); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/FunctionAnalyzer.java b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/FunctionAnalyzer.java index 86f90526fdbb2..f35590b88ceb2 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/FunctionAnalyzer.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/FunctionAnalyzer.java @@ -207,7 +207,7 @@ private static void analyzeBuiltinAggFunction(FunctionName fnName, throw new SemanticException("COUNT_IF does not support DISTINCT", functionCallExpr.getPos()); } - if (fnName.getFunction().equals(FunctionSet.GROUP_CONCAT)) { + if (fnName.getFunction().equals(FunctionSet.GROUP_CONCAT_V2)) { if (functionCallExpr.getChildren().size() - fnParams.getOrderByElemNum() < 2) { throw new SemanticException( "group_concat requires at least one parameter: " + functionCallExpr.toSql(), @@ -866,7 +866,7 @@ public static Pair getArrayAggGroupConcatIntermediateType(String f Type[] argsTypes = new Type[argumentTypes.length]; for (int i = 0; i < argumentTypes.length; ++i) { argsTypes[i] = argumentTypes[i] == Type.NULL ? Type.BOOLEAN : argumentTypes[i]; - if (fnName.equals(FunctionSet.GROUP_CONCAT) && i < argumentTypes.length - isAscOrder.size()) { + if (fnName.equals(FunctionSet.GROUP_CONCAT_V2) && i < argumentTypes.length - isAscOrder.size()) { argsTypes[i] = Type.VARCHAR; } } @@ -908,7 +908,7 @@ private static Function getAdjustedAnalyzedAggregateFunction(ConnectContext sess fn = fn.copy(); fn.setArgsType(argumentTypes); // as accepting various types fn.setIsNullable(false); - } else if (fnName.equals(FunctionSet.ARRAY_AGG) || fnName.equals(FunctionSet.GROUP_CONCAT)) { + } else if (fnName.equals(FunctionSet.ARRAY_AGG) || fnName.equals(FunctionSet.GROUP_CONCAT_V2)) { // move order by expr to node child, and extract is_asc and null_first information. fn = Expr.getBuiltinFunction(fnName, new Type[] {argumentTypes[0]}, Function.CompareMode.IS_NONSTRICT_SUPERTYPE_OF); @@ -925,6 +925,7 @@ private static Function getAdjustedAnalyzedAggregateFunction(ConnectContext sess Pair argsAndIntermediateTypes = getArrayAggGroupConcatIntermediateType(fnName, argumentTypes, isAscOrder); Type[] argsTypes = argsAndIntermediateTypes.first; + fn.setArgsType(argsTypes); // as accepting various types ((AggregateFunction) fn).setIntermediateType(argsAndIntermediateTypes.second); ((AggregateFunction) fn).setIsAscOrder(isAscOrder); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java index 579550e5d80ae..8bb85ed2baad4 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/Utils.java @@ -749,7 +749,7 @@ public static boolean mustGenerateMultiStageAggregate(Operator inputOp, Operator if (children.size() > 1 || children.stream().anyMatch(c -> c.getType().isComplexType())) { return true; } - if (FunctionSet.GROUP_CONCAT.equalsIgnoreCase(fnName) || FunctionSet.AVG.equalsIgnoreCase(fnName)) { + if (FunctionSet.GROUP_CONCAT_V2.equalsIgnoreCase(fnName) || FunctionSet.AVG.equalsIgnoreCase(fnName)) { return true; } else if (FunctionSet.ARRAY_AGG.equalsIgnoreCase(fnName)) { if (children.size() > 1 || children.get(0).getType().isDecimalOfAnyVersion()) { diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/EliminateAggRule.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/EliminateAggRule.java index b196d27317e7b..d34827d331d0d 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/EliminateAggRule.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/EliminateAggRule.java @@ -14,6 +14,7 @@ package com.starrocks.sql.optimizer.rule.transformation; +import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Lists; import com.starrocks.analysis.Expr; import com.starrocks.catalog.Function; @@ -69,6 +70,18 @@ public class EliminateAggRule extends TransformationRule { + private static final Set SUPPORTED_AGGREGATION_FUNCTIONS = + ImmutableSortedSet.orderedBy(String.CASE_INSENSITIVE_ORDER) + .add(FunctionSet.COUNT) + .add(FunctionSet.SUM) + .add(FunctionSet.AVG) + .add(FunctionSet.FIRST_VALUE) + .add(FunctionSet.MAX) + .add(FunctionSet.MIN) + .add(FunctionSet.GROUP_CONCAT) + .add(FunctionSet.GROUP_CONCAT_V2) + .build(); + private EliminateAggRule() { super(RuleType.TF_ELIMINATE_AGG, Pattern.create(OperatorType.LOGICAL_AGGR) .addChildren(Pattern.create(OperatorType.PATTERN_LEAF))); @@ -90,11 +103,7 @@ public boolean check(OptExpression input, OptimizerContext context) { return false; } String fnName = entry.getValue().getFnName(); - if (!(fnName.equals(FunctionSet.SUM) || fnName.equals(FunctionSet.COUNT) || - fnName.equals(FunctionSet.AVG) || - fnName.equals(FunctionSet.FIRST_VALUE) || - fnName.equals(FunctionSet.MAX) || fnName.equals(FunctionSet.MIN) || - fnName.equals(FunctionSet.GROUP_CONCAT))) { + if (!SUPPORTED_AGGREGATION_FUNCTIONS.contains(fnName)) { return false; } } @@ -151,9 +160,7 @@ public List transform(OptExpression input, OptimizerContext conte private ScalarOperator handleAggregationFunction(String fnName, CallOperator callOperator) { if (fnName.equals(FunctionSet.COUNT)) { return rewriteCountFunction(callOperator); - } else if (fnName.equals(FunctionSet.SUM) || fnName.equals(FunctionSet.AVG) || - fnName.equals(FunctionSet.FIRST_VALUE) || fnName.equals(FunctionSet.MAX) || - fnName.equals(FunctionSet.MIN) || fnName.equals(FunctionSet.GROUP_CONCAT)) { + } else if (SUPPORTED_AGGREGATION_FUNCTIONS.contains(fnName)) { return rewriteCastFunction(callOperator); } return callOperator; diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/MultiDistinctByCTERewriter.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/MultiDistinctByCTERewriter.java index 6c239013741b6..a8c3601bf99f9 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/MultiDistinctByCTERewriter.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/MultiDistinctByCTERewriter.java @@ -15,6 +15,7 @@ package com.starrocks.sql.optimizer.rule.transformation; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.starrocks.analysis.BinaryType; @@ -52,6 +53,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import static com.starrocks.catalog.Function.CompareMode.IS_NONSTRICT_SUPERTYPE_OF; @@ -122,6 +124,14 @@ public class MultiDistinctByCTERewriter { private final ScalarOperatorRewriter scalarRewriter = new ScalarOperatorRewriter(); + private static final Set SUPPORTED_DISTINCT_AGG_FUNCTIONS = + ImmutableSortedSet.orderedBy(String.CASE_INSENSITIVE_ORDER) + .add(FunctionSet.COUNT) + .add(FunctionSet.SUM) + .add(FunctionSet.ARRAY_AGG) + .add(FunctionSet.GROUP_CONCAT) + .add(FunctionSet.GROUP_CONCAT_V2) + .build(); public List transformImpl(OptExpression input, OptimizerContext context) { ColumnRefFactory columnRefFactory = context.getColumnRefFactory(); @@ -237,10 +247,8 @@ private LinkedList buildDistinctAggCTEConsume(LogicalAggregationO Map consumeAggCallMap = Maps.newHashMap(); for (ColumnRefOperator distinctAggRef : distinctAggList) { CallOperator aggCallOperator = aggregate.getAggregations().get(distinctAggRef); - if (aggCallOperator.getFnName().equalsIgnoreCase(FunctionSet.COUNT) || - aggCallOperator.getFnName().equalsIgnoreCase(FunctionSet.SUM) || - aggCallOperator.getFnName().equalsIgnoreCase(FunctionSet.ARRAY_AGG) || - aggCallOperator.getFnName().equalsIgnoreCase(FunctionSet.GROUP_CONCAT)) { + String fnName = aggCallOperator.getFnName(); + if (SUPPORTED_DISTINCT_AGG_FUNCTIONS.contains(fnName)) { allCteConsumes.offer(buildCountSumDistinctCTEConsume(distinctAggRef, aggCallOperator, aggregate, cteProduce, factory)); consumeAggCallMap.put(aggCallOperator, distinctAggRef); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/RewriteMultiDistinctRule.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/RewriteMultiDistinctRule.java index e3adf230c5377..17ddca71d5e15 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/RewriteMultiDistinctRule.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/RewriteMultiDistinctRule.java @@ -154,10 +154,7 @@ private boolean useCteToRewrite(OptExpression input, OptimizerContext context) { String fnName = distinctCall.getFnName(); List children = distinctCall.getChildren(); Type type = children.get(0).getType(); - if (type.isComplexType() - || type.isJsonType() - || FunctionSet.GROUP_CONCAT.equalsIgnoreCase(fnName) - || (FunctionSet.ARRAY_AGG.equalsIgnoreCase(fnName) && type.isDecimalOfAnyVersion())) { + if (!canRewriteByMultiFunc(type, fnName)) { canRewriteByMultiFunc = false; break; } @@ -171,6 +168,19 @@ private boolean useCteToRewrite(OptExpression input, OptimizerContext context) { return !canRewriteByMultiFunc; } + private boolean canRewriteByMultiFunc(Type type, String fnName) { + if (type.isComplexType() || type.isJsonType()) { + return false; + } + if (FunctionSet.GROUP_CONCAT_FUNCS.contains(fnName)) { + return false; + } + if (FunctionSet.ARRAY_AGG.equalsIgnoreCase(fnName) && type.isDecimalOfAnyVersion()) { + return false; + } + return true; + } + private boolean isCTEMoreEfficient(OptExpression input, OptimizerContext context, List distinctAggOperatorList) { LogicalAggregationOperator aggOp = input.getOp().cast(); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitAggregateRule.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitAggregateRule.java index 51d2c23bbdafa..b6d04d1a92498 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitAggregateRule.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitAggregateRule.java @@ -132,7 +132,7 @@ private boolean canGenerateTwoStageAggregate(CallOperator distinctCall) { // 3. group_concat distinct or avg distinct is not support two stage aggregate // 4. array_agg with order by clause or decimal distinct col is not support two stage aggregate String fnName = distinctCall.getFnName(); - if (FunctionSet.GROUP_CONCAT.equalsIgnoreCase(fnName) || FunctionSet.AVG.equalsIgnoreCase(fnName)) { + if (FunctionSet.GROUP_CONCAT_V2.equalsIgnoreCase(fnName) || FunctionSet.AVG.equalsIgnoreCase(fnName)) { return false; } else if (FunctionSet.ARRAY_AGG.equalsIgnoreCase(fnName)) { AggregateFunction aggregateFunction = (AggregateFunction) distinctCall.getFunction(); diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitTwoPhaseAggRule.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitTwoPhaseAggRule.java index 5855023514d91..662f46e715fc0 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitTwoPhaseAggRule.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/rule/transformation/SplitTwoPhaseAggRule.java @@ -131,8 +131,7 @@ private CallOperator rewriteDistinctAggFn(CallOperator fnCall) { Expr.getBuiltinFunction(FunctionSet.ARRAY_AGG_DISTINCT, new Type[] {fnCall.getChild(0).getType()}, IS_NONSTRICT_SUPERTYPE_OF), false); } - - } else if (functionName.equals(FunctionSet.GROUP_CONCAT)) { + } else if (functionName.equals(FunctionSet.GROUP_CONCAT_V2)) { // all children of group_concat are constant return fnCall; } else if (functionName.equals(FunctionSet.AVG)) { diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java index 5663fa522bf5d..646ef78323d4c 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java @@ -6519,7 +6519,6 @@ public ParseNode visitAggregationFunctionCall(StarRocksParser.AggregationFunctio NodePosition pos = createPos(context); String functionName; boolean isGroupConcat = false; - boolean isLegacyGroupConcat = false; boolean isDistinct = false; if (context.aggregationFunction().COUNT() != null) { functionName = FunctionSet.COUNT; @@ -6537,48 +6536,18 @@ public ParseNode visitAggregationFunctionCall(StarRocksParser.AggregationFunctio } else if (context.aggregationFunction().GROUP_CONCAT() != null) { functionName = FunctionSet.GROUP_CONCAT; isGroupConcat = true; - isLegacyGroupConcat = SqlModeHelper.check(sqlMode, SqlModeHelper.MODE_GROUP_CONCAT_LEGACY); } else { functionName = FunctionSet.MAX; } + // order by List orderByElements = new ArrayList<>(); if (context.aggregationFunction().ORDER() != null) { orderByElements = visit(context.aggregationFunction().sortItem(), OrderByElement.class); } - - List hints = Lists.newArrayList(); - if (context.aggregationFunction().bracketHint() != null) { - hints = context.aggregationFunction().bracketHint().identifier().stream().map( - RuleContext::getText).collect(Collectors.toList()); - } - if (context.aggregationFunction().setQuantifier() != null) { - isDistinct = context.aggregationFunction().setQuantifier().DISTINCT() != null; - } - - if (isDistinct && CollectionUtils.isEmpty(context.aggregationFunction().expression())) { - throw new ParsingException(PARSER_ERROR_MSG.wrongNumOfArgs(functionName), pos); - } List exprs = visit(context.aggregationFunction().expression(), Expr.class); - if (isGroupConcat && !exprs.isEmpty() && context.aggregationFunction().SEPARATOR() == null) { - if (isLegacyGroupConcat) { - if (exprs.size() == 1) { - Expr sepExpr; - String sep = ", "; - sepExpr = new StringLiteral(sep, pos); - exprs.add(sepExpr); - } - } else { - Expr sepExpr; - String sep = ","; - sepExpr = new StringLiteral(sep, pos); - exprs.add(sepExpr); - } - } + boolean isContainOrderByElements = !orderByElements.isEmpty(); if (!orderByElements.isEmpty()) { int exprSize = exprs.size(); - if (isGroupConcat) { // the last expr of group_concat is the separator - exprSize--; - } for (OrderByElement orderByElement : orderByElements) { Expr by = orderByElement.getExpr(); if (by instanceof IntLiteral) { @@ -6594,6 +6563,34 @@ public ParseNode visitAggregationFunctionCall(StarRocksParser.AggregationFunctio // remove const order-by items orderByElements = orderByElements.stream().filter(x -> !x.getExpr().isConstant()).collect(toList()); } + + // hint + List hints = Lists.newArrayList(); + if (context.aggregationFunction().bracketHint() != null) { + hints = context.aggregationFunction().bracketHint().identifier().stream().map( + RuleContext::getText).collect(Collectors.toList()); + } + + // is distinct + if (context.aggregationFunction().setQuantifier() != null) { + isDistinct = context.aggregationFunction().setQuantifier().DISTINCT() != null; + } + if (isDistinct && CollectionUtils.isEmpty(context.aggregationFunction().expression())) { + throw new ParsingException(PARSER_ERROR_MSG.wrongNumOfArgs(functionName), pos); + } + + // GROUP_CONCAT: use higher version of group_concat if there is order by or distinct clause + if (isGroupConcat && (isDistinct || isContainOrderByElements + || context.aggregationFunction().SEPARATOR() != null)) { + functionName = FunctionSet.GROUP_CONCAT_V2; + if (!exprs.isEmpty() && context.aggregationFunction().SEPARATOR() == null) { + Expr sepExpr; + String sep = ","; + sepExpr = new StringLiteral(sep, pos); + exprs.add(sepExpr); + } + } + if (CollectionUtils.isNotEmpty(orderByElements)) { orderByElements.stream().forEach(e -> exprs.add(e.getExpr())); } diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java index 23c631a1e1fc1..473880d5cb854 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeTestUtil.java @@ -319,9 +319,9 @@ public static StatementBase analyzeSuccess(String originStmt) { Analyzer.analyze(statementBase, connectContext); if (statementBase instanceof QueryStatement) { + String sql = AstToSQLBuilder.toSQL(statementBase); StatementBase viewStatement = - com.starrocks.sql.parser.SqlParser.parse(AstToSQLBuilder.toSQL(statementBase), - connectContext.getSessionVariable()).get(0); + com.starrocks.sql.parser.SqlParser.parse(sql, connectContext.getSessionVariable()).get(0); Analyzer.analyze(viewStatement, connectContext); } diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/AggregateTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/AggregateTest.java index 9670c470821df..c3fca2c8d2a31 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/AggregateTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/AggregateTest.java @@ -2722,7 +2722,7 @@ public void testTableAliasCountDistinctHaving() throws Exception { public void testLegacyGroupConcat() throws Exception { String sql = "select /*+ set_var(sql_mode = GROUP_CONCAT_LEGACY) */ group_concat(v1) from t0"; String plan = getFragmentPlan(sql); - assertContains(plan, "output: group_concat(CAST(1: v1 AS VARCHAR), ', ')"); + assertContains(plan, "output: group_concat(CAST(1: v1 AS VARCHAR))"); sql = "select /*+ set_var('sql_mode' = 'GROUP_CONCAT_LEGACY, ONLY_full_group_by') */ group_concat(v1, '-') from t0"; @@ -2741,7 +2741,7 @@ public void testLegacyGroupConcat() throws Exception { sql = "select /*+ set_var(sql_mode = 68719476768) */ /*+ set_var(sql_mode = 32) */ group_concat(v1, '-') from t0"; plan = getFragmentPlan(sql); - assertContains(plan, "output: group_concat(CAST(1: v1 AS VARCHAR), '-', ',')"); + assertContains(plan, "output: group_concat(CAST(1: v1 AS VARCHAR), '-')"); } @Test diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistinctAggTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistinctAggTest.java index 1b27a0c255329..65067719385a6 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistinctAggTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistinctAggTest.java @@ -72,7 +72,7 @@ void testDistinctConstants() throws Exception { assertContains(plan, "1:AGGREGATE (update serialize)\n" + " | STREAMING\n" + " | output: multi_distinct_count(1, 2, 3, 4), multi_distinct_sum(1), avg(1), " + - "group_concat(DISTINCT '1', '2', ','), array_agg_distinct(1)"); + "group_concat2(DISTINCT '1', '2', ','), array_agg_distinct(1)"); sql = "select count(distinct 1, 2, 3, 4) from t0 group by v2"; plan = getFragmentPlan(sql); assertContains(plan, "3:AGGREGATE (merge finalize)\n" + @@ -91,7 +91,7 @@ void testDistinctConstants() throws Exception { sql = "select group_concat(distinct 1.33) from t0"; plan = getFragmentPlan(sql); assertContains(plan, "2:AGGREGATE (update serialize)\n" + - " | output: group_concat(DISTINCT '1.33', ',')"); + " | output: group_concat2(DISTINCT '1.33', ',')"); sql = "select distinct 111 as id, 111 as id from t0 order by id + 1"; plan = getFragmentPlan(sql); @@ -111,7 +111,7 @@ private static Stream sqlWithDistinctLimit() { " | group by:")); argumentsList.add(Arguments.of("select group_concat(distinct v1, v2) from (select * from t0 limit 2) t", "4:AGGREGATE (merge finalize)\n" + - " | output: group_concat(4: group_concat, ',')\n" + + " | output: group_concat2(4: group_concat2, ',')\n" + " | group by: ")); argumentsList.add(Arguments.of("select count(distinct v1, v2) from (select * from t0 limit 2) t group by 1 + 1", "5:AGGREGATE (merge finalize)\n" + @@ -119,7 +119,7 @@ private static Stream sqlWithDistinctLimit() { " | group by: 4: expr")); argumentsList.add(Arguments.of("select group_concat(distinct v1, v2) from (select * from t0 limit 2) t group by v3", "3:AGGREGATE (update finalize)\n" + - " | output: group_concat(CAST(1: v1 AS VARCHAR), CAST(2: v2 AS VARCHAR), ',')\n" + + " | output: group_concat2(CAST(1: v1 AS VARCHAR), CAST(2: v2 AS VARCHAR), ',')\n" + " | group by: 3: v3")); argumentsList.add(Arguments.of("select count(distinct v1, v2), sum(v1) + max(v2) " + @@ -130,7 +130,7 @@ private static Stream sqlWithDistinctLimit() { argumentsList.add(Arguments.of("select group_concat(distinct v1, v2), sum(v2) + max(v1)" + " from (select * from t0 limit 2) t", "4:AGGREGATE (merge finalize)\n" + - " | output: group_concat(4: group_concat, ','), sum(5: sum), max(6: max)\n" + + " | output: group_concat2(4: group_concat2, ','), sum(5: sum), max(6: max)\n" + " | group by: ")); argumentsList.add(Arguments.of("select count(distinct v1, v2), sum(v1) - min(v2) " + "from (select * from t0 limit 2) t group by 1 + 1", @@ -140,9 +140,7 @@ private static Stream sqlWithDistinctLimit() { argumentsList.add(Arguments.of("select group_concat(distinct v1, v2), sum(v2) - min(v1) " + "from (select * from t0 limit 2) t group by v3", "3:AGGREGATE (update finalize)\n" + - " | output: group_concat(CAST(1: v1 AS VARCHAR), CAST(2: v2 AS VARCHAR), ',')")); - - + " | output: group_concat2(CAST(1: v1 AS VARCHAR), CAST(2: v2 AS VARCHAR), ',')")); argumentsList.add(Arguments.of("select array_agg(distinct v1 order by 1, v3), sum(v2) from t0 " + "group by rollup(v3, abs(v1 + v2))", @@ -173,24 +171,25 @@ private static Stream sqlWithDistinctLimit() { argumentsList.add(Arguments.of("select group_concat(distinct v1 order by 1, v3), sum(v2) from t0 " + "group by rollup(v3, abs(v1 + v2))", "6:AGGREGATE (update finalize)\n" + - " | output: group_concat(CAST(1: v1 AS VARCHAR), ',', 1: v1, 5: expr), sum(7: sum)\n" + + " | output: group_concat2(CAST(1: v1 AS VARCHAR), ',', 1: v1, 5: expr), sum(7: sum)\n" + " | group by: 3: v3, 4: abs, 8: GROUPING_ID")); argumentsList.add(Arguments.of("select /*+set_var(new_planner_agg_stage = 4) */" + "if(length(group_concat(distinct v1, v2 order by 2, v3)) > 10, " + "max(v1), min(v1)), sum(v2) from t0 group by rollup(v3, abs(v1 + v2))", "8:AGGREGATE (merge finalize)\n" + - " | output: group_concat(6: group_concat, ','), max(7: max), min(8: min), sum(9: sum)\n" + + " | output: group_concat2(6: group_concat2, ','), max(7: max), min(8: min), sum(9: sum)\n" + " | group by: 3: v3, 4: abs, 10: GROUPING_ID")); argumentsList.add(Arguments.of("select group_concat(distinct v2), array_agg(distinct v2), " + "count(distinct v2), sum(v3 + v1) from t0 group by rollup(v3, v1);", "6:AGGREGATE (update finalize)\n" + - " | output: group_concat(CAST(2: v2 AS VARCHAR), ','), array_agg(2: v2), count(2: v2), sum(8: sum)")); + " | output: group_concat2(CAST(2: v2 AS VARCHAR), ','), array_agg(2: v2), count(2: v2), sum(8: sum)")); + argumentsList.add(Arguments.of("select group_concat(distinct 1), array_agg(distinct 2), sum(v3) from t0 " + "group by v2, v3", "3:AGGREGATE (merge finalize)\n" + - " | output: group_concat(4: group_concat, '1', ','), array_agg_distinct(5: array_agg), sum(6: sum)")); + " | output: group_concat2(4: group_concat2, '1', ','), array_agg_distinct(5: array_agg), sum(6: sum)")); return argumentsList.stream(); } diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java index 29a6dfcf1c75f..b2f6281e2ac96 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/DistributedEnvPlanWithCostTest.java @@ -1606,7 +1606,7 @@ public void testOneTabletDistinctAgg() throws Exception { String sql = "select sum(id), group_concat(distinct name) from skew_table where id = 1 group by id"; String plan = getFragmentPlan(sql); assertContains(plan, "2:AGGREGATE (update finalize)\n" + - " | output: sum(3: sum), group_concat(2: name, ',')\n" + + " | output: sum(3: sum), group_concat2(2: name, ',')\n" + " | group by: 1: id\n" + " | \n" + " 1:AGGREGATE (update serialize)\n" + diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/MetricTypeTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/MetricTypeTest.java index 4a8f8fa741a61..d0a2b7041f516 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/plan/MetricTypeTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/plan/MetricTypeTest.java @@ -32,7 +32,7 @@ public void testBitmapQuery() throws Exception { "PREAGGREGATION: OFF. Reason: Aggregate Operator not match: COUNT <--> BITMAP_UNION"); starRocksAssert.query("select group_concat(id2) from test.bitmap_table;") - .analysisError("No matching function with signature: group_concat(bitmap"); + .analysisError("group_concat requires first parameter to be of getType() STRING"); starRocksAssert.query("select sum(id2) from test.bitmap_table;").analysisError( "No matching function with signature: sum(bitmap)."); @@ -61,7 +61,7 @@ public void testHLLTypeQuery() throws Exception { "PREAGGREGATION: OFF. Reason: Aggregate Operator not match: COUNT <--> HLL_UNION"); starRocksAssert.query("select group_concat(id2) from test.hll_table;") - .analysisError("No matching function with signature: group_concat(hll"); + .analysisError("group_concat requires first parameter to be of getType() STRING"); starRocksAssert.query("select sum(id2) from test.hll_table;") .analysisError("No matching function with signature: sum(hll)."); diff --git a/gensrc/thrift/InternalService.thrift b/gensrc/thrift/InternalService.thrift index 6da7eb1cf1499..8c013a22a298b 100644 --- a/gensrc/thrift/InternalService.thrift +++ b/gensrc/thrift/InternalService.thrift @@ -320,6 +320,7 @@ struct TQueryOptions { 140: optional string catalog; 141: optional i32 datacache_evict_probability; + 142: optional string default_group_concat_separator; 150: optional map ann_params; 151: optional double pq_refine_factor; diff --git a/test/sql/test_agg_function/R/test_group_concat b/test/sql/test_agg_function/R/test_group_concat index 875c4cd0c6dcf..b077781ffe016 100644 --- a/test/sql/test_agg_function/R/test_group_concat +++ b/test/sql/test_agg_function/R/test_group_concat @@ -1469,11 +1469,11 @@ TomEnglish,王武程咬金语文北京上海 王武程咬金22 -- !result select group_concat(); -- result: -E: (1064, 'Getting analyzing error from line 1, column 7 to line 1, column 20. Detail message: group_concat should have at least one input.') +E: (1064, 'Getting analyzing error from line 1, column 7 to line 1, column 20. Detail message: group_concat requires one or two parameters: group_concat().') -- !result select group_concat() from ss; -- result: -E: (1064, 'Getting analyzing error from line 1, column 7 to line 1, column 20. Detail message: group_concat should have at least one input.') +E: (1064, 'Getting analyzing error from line 1, column 7 to line 1, column 20. Detail message: group_concat requires one or two parameters: group_concat().') -- !result select group_concat(','); -- result: @@ -1512,7 +1512,7 @@ select group_concat("中国",name order by 2, "第一", id separator subject) fr -- !result select group_concat("中国" order by "第一" separator 1) from ss; -- result: -E: (1064, "Getting analyzing error at line 1, column 49. Detail message: group_concat requires separator to be of getType() STRING: group_concat('中国', 1).") +E: (1064, "Getting analyzing error at line 1, column 49. Detail message: group_concat requires separator to be of getType() STRING: group_concat2('中国', 1).") -- !result select group_concat( order by score) from ss order by 1; -- result: @@ -1524,7 +1524,7 @@ E: (1064, "Getting syntax error at line 1, column 30. Detail message: Unexpected -- !result select group_concat([1,2]) from ss; -- result: -E: (1064, 'Getting analyzing error from line 1, column 7 to line 1, column 25. Detail message: No matching function with signature: group_concat(array, varchar).') +E: (1064, 'Getting analyzing error from line 1, column 20 to line 1, column 24. Detail message: group_concat requires first parameter to be of getType() STRING: group_concat([1,2]).') -- !result select group_concat(json_object("2:3")) from ss; -- result: @@ -1532,7 +1532,7 @@ select group_concat(json_object("2:3")) from ss; -- !result select group_concat(map(2,3)) from ss; -- result: -E: (1064, 'Getting analyzing error from line 1, column 7 to line 1, column 28. Detail message: No matching function with signature: group_concat(map, varchar).') +E: (1064, 'Getting analyzing error from line 1, column 20 to line 1, column 27. Detail message: group_concat requires first parameter to be of getType() STRING: group_concat(map{2:3}).') -- !result select group_concat(null); -- result: @@ -1706,12 +1706,12 @@ select /*+ set_var(sql_mode = 68719476736) */ group_concat( value, '-' ) from t1 -- result: fruit-fruit-fruit-fruit-fruit-fruit -- !result -select /*+ set_var(sql_mode = 'GROUP_CONCAT_LEGACY') */ /*+ set_var(sql_mode = 'ONLY_FULL_GROUP_BY') */ id, group_concat( value, '-' ) from t1 group by id order by id; +select /*+ set_var(sql_mode = 'ONLY_FULL_GROUP_BY') */ id, group_concat( value, '-' ) from t1 group by id order by id; -- result: -1 fruit-,fruit-,fruit- -2 fruit-,fruit-,fruit- +1 fruit-fruit-fruit +2 fruit-fruit-fruit -- !result -select /*+ set_var(sql_mode = 68719476736) */ /*+ set_var(sql_mode = 32) */ group_concat( value ) from t1; +select /*+ set_var(sql_mode = 32) */ group_concat( value ) from t1; -- result: fruit,fruit,fruit,fruit,fruit,fruit --- !result +-- !result \ No newline at end of file diff --git a/test/sql/test_agg_function/T/test_group_concat b/test/sql/test_agg_function/T/test_group_concat index 528547493c19d..e203d7b869e46 100644 --- a/test/sql/test_agg_function/T/test_group_concat +++ b/test/sql/test_agg_function/T/test_group_concat @@ -334,5 +334,5 @@ select /*+ set_var(sql_mode = 68719476736) */ group_concat( value, '-' ) from t1 -- new result -select /*+ set_var(sql_mode = 'GROUP_CONCAT_LEGACY') */ /*+ set_var(sql_mode = 'ONLY_FULL_GROUP_BY') */ id, group_concat( value, '-' ) from t1 group by id order by id; -select /*+ set_var(sql_mode = 68719476736) */ /*+ set_var(sql_mode = 32) */ group_concat( value ) from t1; +select /*+ set_var(sql_mode = 'ONLY_FULL_GROUP_BY') */ id, group_concat( value, '-' ) from t1 group by id order by id; +select /*+ set_var(sql_mode = 32) */ group_concat( value ) from t1;