diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java index f4327d86b2..cce7f63045 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java @@ -109,7 +109,7 @@ public class ChildrenPropertiesRegulator extends PlanVisitor { return false; } if (!agg.getAggregateParam().canBeBanned) { - return true; + return visit(agg, context); } // forbid one phase agg on distribute if (agg.getAggMode() == AggMode.INPUT_TO_RESULT && children.get(0).getPlan() instanceof PhysicalDistribute) { @@ -175,9 +175,7 @@ public class ChildrenPropertiesRegulator extends PlanVisitor { } } // process must shuffle - visit(agg, context); - // process agg - return true; + return visit(agg, context); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java index 7ca8637446..ae7bb232e8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java @@ -24,6 +24,7 @@ import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.OrderExpression; import org.apache.doris.nereids.trees.expressions.WindowExpression; import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction; +import org.apache.doris.nereids.trees.expressions.functions.agg.Count; import org.apache.doris.nereids.trees.expressions.functions.generator.TableGeneratingFunction; import org.apache.doris.nereids.trees.expressions.functions.scalar.GroupingScalarFunction; import org.apache.doris.nereids.trees.expressions.typecoercion.TypeCheckResult; @@ -148,11 +149,13 @@ public class CheckAnalysis implements AnalysisRuleFactory { if (func.arity() <= 1) { continue; } - for (int i = 1; i < func.arity(); i++) { - if (!func.child(i).getInputSlots().isEmpty() && !(func.child(i) instanceof OrderExpression)) { - // think about group_concat(distinct col_1, ',') - distinctMultiColumns = true; - break; + if (func instanceof Count) { + for (int i = 1; i < func.arity(); i++) { + if (!func.child(i).getInputSlots().isEmpty() && !(func.child(i) instanceof OrderExpression)) { + // think about group_concat(distinct col_1, ',') + distinctMultiColumns = true; + break; + } } } if (distinctMultiColumns) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java index 3f690ad3c3..8660034a2b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java @@ -2048,10 +2048,12 @@ public class AggregateStrategies implements ImplementationRuleFactory { if (func.arity() <= 1) { continue; } - for (int i = 1; i < func.arity(); i++) { - // think about group_concat(distinct col_1, ',') - if (!(func.child(i) instanceof OrderExpression) && !func.child(i).getInputSlots().isEmpty()) { - return false; + if (func instanceof Count) { + for (int i = 1; i < func.arity(); i++) { + // think about group_concat(distinct col_1, ',') + if (!(func.child(i) instanceof OrderExpression) && !func.child(i).getInputSlots().isEmpty()) { + return false; + } } } } 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 d01900ef88..3065713cf5 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_13 -- +1 2 +1 2 + -- !select_group_concat_order_by_all_data -- 1 1 1 1 1 11 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 522d66ed64..57056c7318 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 @@ -16,6 +16,9 @@ // under the License. suite("test_group_concat", "query,p0,arrow_flight_sql") { + + sql "set enable_fallback_to_original_planner=false" + qt_select """ SELECT group_concat(k6) FROM test_query_db.test where k6='false' """ @@ -79,6 +82,16 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") { b2; """ + // test SPLIT_MULTI_DISTINCT could work right with can not be banned aggregation + qt_select_13 """ + select + group_concat( distinct b1, cast(b2 as varchar)), group_concat( distinct b3, '?') + from + table_group_concat + group by + b2; + """ + sql """ drop table table_group_concat """ sql """create table table_group_concat ( b1 varchar(10) not null, b2 int not null, b3 varchar(10) not null ) ENGINE=OLAP