This is an automated email from the ASF dual-hosted git repository.
morrySnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 052880662b0 [fix](agg) Fix distinct aggregate argument grouping and
strategy selection (#65368)
052880662b0 is described below
commit 052880662b061000694f338d682c741f947353ac
Author: feiniaofeiafei <[email protected]>
AuthorDate: Tue Jul 14 15:43:38 2026 +0800
[fix](agg) Fix distinct aggregate argument grouping and strategy selection
(#65368)
### What problem does this PR solve?
Problem Summary:
The Nereids distinct aggregate planning logic had several correctness
issues:
1. `CheckMultiDistinct` determined distinct groups from all aggregate
arguments instead of the semantic DISTINCT arguments. This incorrectly
treated separators and ORDER BY expressions as deduplication keys and
mishandled aggregates whose DISTINCT argument sets were equivalent but
ordered differently, such as `COUNT(DISTINCT a, b)` and `COUNT(DISTINCT b,
a)`.
2. `GROUP_CONCAT` accepted a column expression as its separator. During
distinct aggregate rewriting, this extra input could incorrectly participate
in deduplication even though DISTINCT semantics only apply to the value
argument.
3. Forced `agg_phase=3/4` could override `GROUP_CONCAT(DISTINCT ...
ORDER BY ...)`'s requirement to use the multi-distinct implementation.
Conversely, early logical splitting could prevent forced three/four-phase
aggregation from being honored.
4. `AggregateUtils#getDistinctNamedExpr` collected all aggregate
arguments rather than only semantic DISTINCT arguments.
This PR:
- Compares DISTINCT aggregates by their semantic DISTINCT argument groups.
- Treats equivalent argument sets as one distinct group.
- Requires the separator of `GROUP_CONCAT` and
`MULTI_DISTINCT_GROUP_CONCAT` to be a constant.
- Introduces explicit strategy selection between logical splitting,
multi-distinct aggregation, and Cascades multi-phase splitting.
- Preserves `mustUseMultiDistinctAgg()` when `agg_phase` is forced.
- Lets `agg_phase=3/4` retain the original DISTINCT aggregate for Cascades
phase planning.
- Rejects queries that combine `GROUP_CONCAT(DISTINCT ... ORDER BY ...)`
with a multi-argument `COUNT(DISTINCT ...)`, because their required aggregation
strategies are incompatible.
- Uses `AggregateFunction#getDistinctArguments()` when collecting distinct
deduplication keys.
### Release note
`GROUP_CONCAT` and `MULTI_DISTINCT_GROUP_CONCAT` now require the
separator argument to be a constant.
Queries combining `GROUP_CONCAT(DISTINCT ... ORDER BY ...)` with a
multi-argument `COUNT(DISTINCT ...)` are rejected with a clear analysis
error.
---
.../rules/implementation/SplitAggMultiPhase.java | 2 +-
.../SplitAggMultiPhaseWithoutGbyKey.java | 2 +-
.../nereids/rules/rewrite/CheckMultiDistinct.java | 50 ++--------------
.../rules/rewrite/DistinctAggStrategySelector.java | 3 +-
.../rules/rewrite/DistinctAggregateRewriter.java | 67 ++++++++++++++++------
.../expressions/functions/agg/GroupConcat.java | 8 +++
.../functions/agg/MultiDistinctGroupConcat.java | 8 +++
.../apache/doris/nereids/util/AggregateUtils.java | 20 ++++++-
.../rewrite/DistinctAggregateRewriterTest.java | 13 +++--
.../agg_strategy/distinct_agg_rewriter.out | 9 +++
.../distinct_agg_strategy_selector.out | 15 +++++
.../agg_strategy/distinct_agg_rewriter.groovy | 23 +++++++-
.../distinct_agg_strategy_selector.groovy | 4 ++
.../query_p0/aggregate/agg_group_concat.groovy | 9 +++
.../query_p0/group_concat/test_group_concat.groovy | 2 +-
15 files changed, 158 insertions(+), 77 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhase.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhase.java
index 9a5b9ad412a..390c3af6e04 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhase.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhase.java
@@ -76,7 +76,7 @@ public class SplitAggMultiPhase extends SplitAggBaseRule
implements Implementati
return ImmutableList.of(
logicalAggregate()
.when(agg -> !agg.getGroupByExpressions().isEmpty())
- .when(agg -> agg.getDistinctArguments().size() == 1 ||
agg.distinctFuncNum() == 1)
+ .when(agg ->
AggregateUtils.distinctArgumentGroupCountUpToTwo(agg) == 1)
.thenApplyMulti(this::rewrite)
.toRule(RuleType.SPLIT_AGG_MULTI_PHASE)
);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhaseWithoutGbyKey.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhaseWithoutGbyKey.java
index 28ddb8ff718..36862448313 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhaseWithoutGbyKey.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/SplitAggMultiPhaseWithoutGbyKey.java
@@ -76,7 +76,7 @@ public class SplitAggMultiPhaseWithoutGbyKey extends
SplitAggBaseRule implements
return ImmutableList.of(
logicalAggregate()
.when(agg -> agg.getGroupByExpressions().isEmpty())
- .when(agg -> agg.getDistinctArguments().size() == 1 ||
agg.distinctFuncNum() == 1)
+ .when(agg ->
AggregateUtils.distinctArgumentGroupCountUpToTwo(agg) == 1)
.thenApplyMulti(ctx -> rewrite(ctx.root,
ctx.cascadesContext))
.toRule(RuleType.SPLIT_AGG_MULTI_PHASE_WITHOUT_GBY_KEY)
);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckMultiDistinct.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckMultiDistinct.java
index d9c953444ba..0f23dcd1a7d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckMultiDistinct.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/CheckMultiDistinct.java
@@ -20,20 +20,12 @@ package org.apache.doris.nereids.rules.rewrite;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
-import org.apache.doris.nereids.trees.expressions.OrderExpression;
-import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
-import
org.apache.doris.nereids.trees.expressions.functions.agg.SupportMultiDistinct;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.util.AggregateUtils;
/**
- * If there are multiple distinct aggregate functions that cannot
- * be transformed into multi_distinct, an error is reported.
- * The following functions can be transformed into multi_distinct:
- * - count -> MULTI_DISTINCT_COUNT
- * - sum -> MULTI_DISTINCT_SUM
- * - avg -> MULTI_DISTINCT_AVG
- * - group_concat -> MULTI_DISTINCT_GROUP_CONCAT
+ * Check that previous aggregate rewrite rules have already handled multiple
distinct argument groups.
*/
public class CheckMultiDistinct extends OneRewriteRuleFactory {
@Override
@@ -42,43 +34,9 @@ public class CheckMultiDistinct extends
OneRewriteRuleFactory {
}
private LogicalAggregate checkDistinct(LogicalAggregate<? extends Plan>
aggregate) {
- if (aggregate.getDistinctArguments().size() > 1) {
-
- for (AggregateFunction func : aggregate.getAggregateFunctions()) {
- if (func.isDistinct() && !(func instanceof
SupportMultiDistinct)) {
- throw new AnalysisException(func.toString() + " can't
support multi distinct.");
- }
- }
- }
-
- boolean distinctMultiColumns = false;
- for (AggregateFunction func : aggregate.getAggregateFunctions()) {
- if (!func.isDistinct()) {
- continue;
- }
- 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 (distinctMultiColumns) {
- break;
- }
- }
-
- long distinctFunctionNum = 0;
- for (AggregateFunction aggregateFunction :
aggregate.getAggregateFunctions()) {
- distinctFunctionNum += aggregateFunction.isDistinct() ? 1 : 0;
- }
-
- if (distinctMultiColumns && distinctFunctionNum > 1) {
+ if (AggregateUtils.distinctArgumentGroupCountUpToTwo(aggregate) > 1) {
throw new AnalysisException(
- "The query contains multi count distinct or sum distinct,
each can't have multi columns");
+ "Multiple DISTINCT aggregate functions with different
argument lists are not supported.");
}
return aggregate;
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggStrategySelector.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggStrategySelector.java
index de83915c843..817460a92fe 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggStrategySelector.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggStrategySelector.java
@@ -103,11 +103,12 @@ public class DistinctAggStrategySelector extends
DefaultPlanRewriter<DistinctSel
// not process：
// count(distinct a,b);
// count(distinct a), sum(distinct a);
+ // count(distinct a,b), count(distinct b,a);
// count(distinct a)
// process:
// count(distinct a,b), count(distinct a,c)
// count(distinct a), sum(distinct b)
- if (agg.distinctFuncNum() < 2 || agg.getDistinctArguments().size() <
2) {
+ if (AggregateUtils.distinctArgumentGroupCountUpToTwo(agg) <= 1) {
return agg;
}
if (shouldUseMultiDistinct(agg)) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriter.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriter.java
index 832641add2d..b6eb9470615 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriter.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriter.java
@@ -21,6 +21,7 @@ import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.DistributionInfo;
import org.apache.doris.catalog.HashDistributionInfo;
import org.apache.doris.catalog.OlapTable;
+import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.rewrite.StatsDerive.DeriveContext;
@@ -83,6 +84,13 @@ public class DistinctAggregateRewriter implements
RewriteRuleFactory {
private static final Set<Class<? extends AggregateFunction>>
supportSplitOtherFunctions = ImmutableSet.of(
Sum.class, Min.class, Max.class, Count.class, Sum0.class,
AnyValue.class);
+ enum Strategy {
+ SPLIT_IN_REWRITE, MULTI_STRATEGY, SPLIT_IN_CASCADES
+ }
+
+ private static final String errorString = "Unsupported query:
GROUP_CONCAT(DISTINCT ... ORDER BY ...)"
+ + " cannot be used together with a multi-argument COUNT(DISTINCT
...).";
+
@Override
public List<Rule> buildRules() {
return ImmutableList.of(
@@ -92,25 +100,41 @@ public class DistinctAggregateRewriter implements
RewriteRuleFactory {
.thenApply(ctx -> rewrite(ctx.root,
ctx.connectContext))
.toRule(RuleType.DISTINCT_AGGREGATE_SPLIT),
logicalAggregate()
- .when(agg -> agg.getGroupByExpressions().isEmpty()
- && agg.mustUseMultiDistinctAgg() &&
!AggregateUtils.containsCountDistinctMultiExpr(agg))
- .then(this::convertToMultiDistinct)
+ .when(agg -> agg.getGroupByExpressions().isEmpty() &&
agg.mustUseMultiDistinctAgg())
+ .then(agg -> {
+ // count(distinct a,b) cannot use multi_distinct
+ if
(AggregateUtils.containsCountDistinctMultiExpr(agg)) {
+ throw new AnalysisException(errorString);
+ }
+ return convertToMultiDistinct(agg);
+ })
.toRule(RuleType.PROCESS_SCALAR_AGG_MUST_USE_MULTI_DISTINCT)
);
}
@VisibleForTesting
- boolean shouldUseMultiDistinct(LogicalAggregate<? extends Plan> aggregate)
{
+ Strategy chooseStrategy(LogicalAggregate<? extends Plan> aggregate) {
// count(distinct a,b) cannot use multi_distinct
- if (AggregateUtils.containsCountDistinctMultiExpr(aggregate)) {
- return false;
- }
- if (aggregate.mustUseMultiDistinctAgg()) {
- return true;
+ boolean mustSplit =
AggregateUtils.containsCountDistinctMultiExpr(aggregate);
+ boolean mustUseMulti = aggregate.mustUseMultiDistinctAgg();
+ if (mustSplit && mustUseMulti) {
+ throw new AnalysisException(errorString);
}
ConnectContext ctx = ConnectContext.get();
+ if (mustSplit) {
+ if (ctx.getSessionVariable().aggPhase == 3 ||
ctx.getSessionVariable().aggPhase == 4) {
+ return Strategy.SPLIT_IN_CASCADES;
+ }
+ return Strategy.SPLIT_IN_REWRITE;
+ }
+ if (mustUseMulti) {
+ return Strategy.MULTI_STRATEGY;
+ }
if (ctx.getSessionVariable().aggPhase == 1 ||
ctx.getSessionVariable().aggPhase == 2) {
- return true;
+ return Strategy.MULTI_STRATEGY;
+ }
+ if (ctx.getSessionVariable().aggPhase == 3 ||
ctx.getSessionVariable().aggPhase == 4) {
+ return Strategy.SPLIT_IN_CASCADES;
}
if (aggregate.getStats() == null || aggregate.child().getStats() ==
null) {
StatsDerive derive = new StatsDerive(false);
@@ -120,12 +144,12 @@ public class DistinctAggregateRewriter implements
RewriteRuleFactory {
Statistics aggChildStats = aggregate.child().getStats();
Set<Expression> dstArgs = aggregate.getDistinctArguments();
if (isDistinctKeySatisfyDistribution(aggregate)) {
- return false;
+ return Strategy.SPLIT_IN_REWRITE;
}
// has unknown statistics, split to bottom and top agg
if
(AggregateUtils.hasUnknownStatistics(aggregate.getGroupByExpressions(),
aggChildStats)
|| AggregateUtils.hasUnknownStatistics(dstArgs,
aggChildStats)) {
- return true;
+ return Strategy.MULTI_STRATEGY;
}
double gbyNdv = aggStats.getRowCount();
@@ -138,8 +162,12 @@ public class DistinctAggregateRewriter implements
RewriteRuleFactory {
double inputRows = aggChildStats.getRowCount();
// group by key ndv is low, distinct key ndv is high, multi_distinct
is better
// otherwise split to bottom and top agg
- return gbyNdv < inputRows * AggregateUtils.LOW_CARDINALITY_THRESHOLD
- && dstNdv > inputRows *
AggregateUtils.HIGH_CARDINALITY_THRESHOLD;
+ if (gbyNdv < inputRows * AggregateUtils.LOW_CARDINALITY_THRESHOLD
+ && dstNdv > inputRows *
AggregateUtils.HIGH_CARDINALITY_THRESHOLD) {
+ return Strategy.MULTI_STRATEGY;
+ } else {
+ return Strategy.SPLIT_IN_REWRITE;
+ }
}
private boolean isDistinctKeySatisfyDistribution(LogicalAggregate<?
extends Plan> aggregate) {
@@ -249,14 +277,15 @@ public class DistinctAggregateRewriter implements
RewriteRuleFactory {
if (aggregate.distinctFuncNum() == 0) {
return null;
}
- if (ctx.getSessionVariable().aggPhase == 3 ||
ctx.getSessionVariable().aggPhase == 4) {
- return null;
- }
- if (shouldUseMultiDistinct(aggregate)) {
+ Strategy strategy = chooseStrategy(aggregate);
+ if (strategy == Strategy.MULTI_STRATEGY) {
return convertToMultiDistinct(aggregate);
- } else {
+ } else if (strategy == Strategy.SPLIT_IN_REWRITE) {
return splitDistinctAgg(aggregate);
+ } else if (strategy == Strategy.SPLIT_IN_CASCADES) {
+ return null;
}
+ return null;
}
private Plan convertToMultiDistinct(LogicalAggregate<? extends Plan>
aggregate) {
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 8003d687380..1d1205aea8c 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
@@ -201,4 +201,12 @@ public class GroupConcat extends NullableAggregateFunction
}
return firstOrderExrIndex;
}
+
+ @Override
+ public void checkLegalityBeforeTypeCoercion() {
+ if (nonOrderArguments == 2 && !getArgument(1).isConstant()) {
+ throw new AnalysisException(
+ "group_concat requires separator must be a constant : " +
this.toSql());
+ }
+ }
}
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 5fc78899a72..8f1f6debe80 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
@@ -140,4 +140,12 @@ public class MultiDistinctGroupConcat extends
NullableAggregateFunction
}
return firstOrderExrIndex;
}
+
+ @Override
+ public void checkLegalityBeforeTypeCoercion() {
+ if (nonOrderArguments == 2 && !getArgument(1).isConstant()) {
+ throw new AnalysisException(
+ "multi_distinct_group_concat requires separator must be a
constant : " + this.toSql());
+ }
+ }
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/AggregateUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/AggregateUtils.java
index c181af415fd..947da096185 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/AggregateUtils.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/AggregateUtils.java
@@ -30,6 +30,7 @@ import
org.apache.doris.nereids.trees.expressions.functions.agg.SupportMultiDist
import org.apache.doris.nereids.trees.expressions.functions.scalar.If;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.algebra.Aggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.statistics.ColumnStatistic;
@@ -147,6 +148,23 @@ public class AggregateUtils {
expr instanceof Count && ((Count) expr).isDistinct() &&
expr.arity() > 1);
}
+ /** count agg function distinct group, up to 2*/
+ public static int distinctArgumentGroupCountUpToTwo(Aggregate<? extends
Plan> aggregate) {
+ Set<Expression> distinctArgumentGroup = null;
+ for (AggregateFunction aggregateFunction :
aggregate.getAggregateFunctions()) {
+ if (!aggregateFunction.isDistinct()) {
+ continue;
+ }
+ Set<Expression> currentGroup =
ImmutableSet.copyOf(aggregateFunction.getDistinctArguments());
+ if (distinctArgumentGroup == null) {
+ distinctArgumentGroup = currentGroup;
+ } else if (!distinctArgumentGroup.equals(currentGroup)) {
+ return 2;
+ }
+ }
+ return distinctArgumentGroup == null ? 0 : 1;
+ }
+
/**getAllKeySet*/
public static Set<NamedExpression> getAllKeySet(LogicalAggregate<? extends
Plan> aggregate) {
Set<NamedExpression> distinctArguments =
getDistinctNamedExpr(aggregate);
@@ -169,7 +187,7 @@ public class AggregateUtils {
public static Set<NamedExpression> getDistinctNamedExpr(LogicalAggregate<?
extends Plan> aggregate) {
return aggregate.getAggregateFunctions().stream()
.filter(AggregateFunction::isDistinct)
- .flatMap(aggFunc -> aggFunc.getArguments().stream())
+ .flatMap(aggFunc -> aggFunc.getDistinctArguments().stream())
.filter(NamedExpression.class::isInstance)
.map(NamedExpression.class::cast)
.collect(ImmutableSet.toImmutableSet());
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriterTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriterTest.java
index 3bfb42918a1..c9d9c05a071 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriterTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DistinctAggregateRewriterTest.java
@@ -20,6 +20,7 @@ package org.apache.doris.nereids.rules.rewrite;
import org.apache.doris.catalog.DistributionInfo;
import org.apache.doris.catalog.HashDistributionInfo;
import
org.apache.doris.nereids.rules.analysis.LogicalSubQueryAliasToLogicalProject;
+import
org.apache.doris.nereids.rules.rewrite.DistinctAggregateRewriter.Strategy;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.Count;
@@ -245,7 +246,7 @@ public class DistinctAggregateRewriterTest extends
TestWithFeService implements
}
@Test
- void testShouldUseMultiDistinctWithoutStatsSatisfyDistribution() throws
Exception {
+ void testChooseStrategyWithoutStatsSatisfyDistribution() throws Exception {
DistinctAggregateRewriter rewriter =
DistinctAggregateRewriter.INSTANCE;
LogicalAggregate<? extends Plan> aggregate = getLogicalAggregate(
"select bb, count(distinct aa) from "
@@ -261,11 +262,11 @@ public class DistinctAggregateRewriterTest extends
TestWithFeService implements
((AbstractPlan) child).setStatistics(new Statistics(10000, colStats));
aggregate.setStatistics(new Statistics(100, ImmutableMap.of()));
- Assertions.assertFalse(rewriter.shouldUseMultiDistinct(aggregate));
+ Assertions.assertEquals(Strategy.SPLIT_IN_REWRITE,
rewriter.chooseStrategy(aggregate));
}
@Test
- void testShouldUseMultiDistinctWithStatsSelected() throws Exception {
+ void testChooseStrategyWithStatsSelected() throws Exception {
DistinctAggregateRewriter rewriter = new DistinctAggregateRewriter();
LogicalAggregate<? extends Plan> aggregate = getLogicalAggregate(
"select b, count(distinct a) from test.distinct_agg_split_t
group by b"
@@ -279,11 +280,11 @@ public class DistinctAggregateRewriterTest extends
TestWithFeService implements
((AbstractPlan) child).setStatistics(new Statistics(100000, colStats));
aggregate.setStatistics(new Statistics(240, ImmutableMap.of()));
- Assertions.assertFalse(rewriter.shouldUseMultiDistinct(aggregate));
+ Assertions.assertEquals(Strategy.SPLIT_IN_REWRITE,
rewriter.chooseStrategy(aggregate));
}
@Test
- void testShouldUseMultiDistinctWithPartitionTable() {
+ void testChooseStrategyWithPartitionTable() {
DistinctAggregateRewriter rewriter =
DistinctAggregateRewriter.INSTANCE;
LogicalAggregate<? extends Plan> aggregate = getLogicalAggregate(
"select count(distinct record_id) from sales_records group by
sale_date;"
@@ -297,7 +298,7 @@ public class DistinctAggregateRewriterTest extends
TestWithFeService implements
((AbstractPlan) child).setStatistics(new Statistics(10000, colStats));
aggregate.setStatistics(new Statistics(100, ImmutableMap.of()));
- Assertions.assertTrue(rewriter.shouldUseMultiDistinct(aggregate));
+ Assertions.assertEquals(Strategy.MULTI_STRATEGY,
rewriter.chooseStrategy(aggregate));
}
@Test
diff --git
a/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.out
b/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.out
index 18088719ce3..d051d2efe37 100644
---
a/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.out
+++
b/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.out
@@ -38,3 +38,12 @@ PhysicalResultSink
--------hashAgg[LOCAL]
----------PhysicalOlapScan[t1000_2]
+-- !agg_phase3 --
+PhysicalResultSink
+--PhysicalDistribute[DistributionSpecGather]
+----hashAgg[DISTINCT_GLOBAL]
+------hashAgg[GLOBAL]
+--------PhysicalDistribute[DistributionSpecHash]
+----------hashAgg[LOCAL]
+------------PhysicalTVFRelation
+
diff --git
a/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.out
b/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.out
index 466c05a97e2..d67a3927da2 100644
---
a/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.out
+++
b/regression-test/data/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.out
@@ -69,3 +69,18 @@ PhysicalResultSink
--------hashAgg[LOCAL]
----------PhysicalOlapScan[t1000]
+-- !count_distinct_group --
+5 5
+
+-- !count_distinct_group_with_gby --
+1 1
+1 1
+1 1
+1 1
+1 1
+1 1
+1 1
+1 1
+1 1
+1 1
+
diff --git
a/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.groovy
b/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.groovy
index 9ad53f9208a..33fb4f62db3 100644
---
a/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.groovy
+++
b/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_rewriter.groovy
@@ -35,4 +35,25 @@ suite("distinct_agg_rewriter") {
select count(distinct d_200) from t1000_2 group by b_5;"""
qt_use_multi_distinct """explain shape plan
select count(distinct d_200) from t1000_2 group by a_1;"""
-}
\ No newline at end of file
+
+ sql "set agg_phase=4"
+ sql "select group_concat(distinct dst_key1 order by dst_key2) from
t_gbykey_10_dstkey_10_1000_id group by gby_key"
+ test {
+ sql """select number % 2, count(distinct cast(number as varchar),
cast(number as varchar)),
+ group_concat(distinct cast(number as varchar) order by number + 1)
from numbers('number'='10') group by number % 2"""
+ exception "Unsupported query"
+ }
+
+ test {
+ sql """select count(distinct cast(number as varchar), cast(number as
varchar)),
+ group_concat(distinct cast(number as varchar) order by number + 1)
from numbers('number'='10')"""
+ exception "Unsupported query"
+ }
+
+ // expect to have 3 HashAgg instead of 4 HashAgg
+ sql "set agg_phase=3"
+ qt_agg_phase3 """explain shape plan
+ select number % 2, count(distinct number)
+ from numbers('number'='10')
+ group by number % 2;"""
+}
diff --git
a/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.groovy
b/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.groovy
index aec45e0d50b..88467c999ba 100644
---
a/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.groovy
+++
b/regression-test/suites/nereids_rules_p0/agg_strategy/distinct_agg_strategy_selector.groovy
@@ -58,4 +58,8 @@ suite("distinct_agg_strategy_selector") {
contains "multi_distinct_count"
}
sql "set multi_distinct_strategy=0 "
+
+ // test
+ order_qt_count_distinct_group "select count(distinct a_1,b_5),
count(distinct b_5,a_1) from t1000;"
+ order_qt_count_distinct_group_with_gby "select count(distinct a_1,b_5),
count(distinct b_5,a_1) from t1000 group by c_10;"
}
\ No newline at end of file
diff --git a/regression-test/suites/query_p0/aggregate/agg_group_concat.groovy
b/regression-test/suites/query_p0/aggregate/agg_group_concat.groovy
index 861042750c2..a05a9edb271 100644
--- a/regression-test/suites/query_p0/aggregate/agg_group_concat.groovy
+++ b/regression-test/suites/query_p0/aggregate/agg_group_concat.groovy
@@ -123,4 +123,13 @@ suite("agg_group_concat") {
insert into test_distinct_multi
values(1,2,3,'abc','2024-01-02'),(1,2,4,'abc','2024-01-03'),(2,2,4,'abcd','2024-01-02'),(1,2,3,'abcd','2024-01-04'),(1,2,4,'eee','2024-02-02'),(2,2,4,'abc','2024-01-02');
"""
qt_test "select group_concat( distinct d order by d) from
test_distinct_multi order by 1; "
+
+ test {
+ sql "select multi_distinct_GROUP_CONCAT(a, c) from test_distinct_multi
group by b; "
+ exception "separator must be a constant"
+ }
+ test {
+ sql "select GROUP_CONCAT(a, c order by c) from test_distinct_multi
group by b; "
+ exception "separator must be a constant"
+ }
}
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 2e8e6f8916d..05c3f2d2e2a 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
@@ -91,7 +91,7 @@ suite("test_group_concat", "query,p0,arrow_flight_sql") {
// 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, '?')
+ group_concat( distinct b1, ','), group_concat( distinct b3,
'?')
from
table_group_concat
group by
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
