Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.9k
[improvement](nereids) Support CTE split for DISTINCT aggregate functions#65664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
f3465439d283d06d0ac7348082709051770File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -247,6 +247,9 @@ private static Set<Expression> collectNeedToSlotArgsOfGroupingScalarFuncAndAggFu | ||
| ImmutableSet.Builder<Expression> argumentsOfAggregateFunctionBuilder = ImmutableSet.builder(); | ||
| for (AggregateFunction function : aggregateFunctions) { | ||
| for (Expression arg : function.getArguments()) { | ||
| if (arg.isConstant()) { | ||
morrySnow marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| continue; | ||
| } | ||
| if (arg instanceof OrderExpression) { | ||
| argumentsOfAggregateFunctionBuilder.add(arg.child(0)); | ||
| } else { | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -119,19 +119,20 @@ public Plan visitLogicalAggregate(LogicalAggregate<? extends Plan> agg, Distinct | ||
| } | ||
| private boolean shouldUseMultiDistinct(LogicalAggregate<? extends Plan> agg) { | ||
| boolean mustUseCte = AggregateUtils.containsCountDistinctMultiExpr(agg); | ||
| boolean mustUseCte = AggregateUtils.containsNotSupportMultiDistinctFunction(agg); | ||
morrySnow marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. morrySnow marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| boolean mustUseMulti = agg.getSourceRepeat().isPresent(); | ||
| if (mustUseCte && mustUseMulti) { | ||
morrySnow marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| throw new AnalysisException( | ||
| "Unsupported query: GROUPING SETS/ROLLUP/CUBE cannot be used with a combination of " | ||
| + "multi-column COUNT(DISTINCT) and other COUNT(DISTINCT) expressions.\n\n" | ||
| "Unsupported query: GROUPING SETS/ROLLUP/CUBE cannot be used with multiple DISTINCT " | ||
| + "argument groups when any DISTINCT aggregate does not support multi-distinct execution.\n\n" | ||
| + "Unsupported scenarios:\n" | ||
| + "• ARRAY_AGG(DISTINCT a) with ARRAY_AGG(DISTINCT b) + GROUPING\n" | ||
| + "• COUNT(DISTINCT a, b) with COUNT(DISTINCT a) + GROUPING\n" | ||
| + "• COUNT(DISTINCT a, b) with COUNT(DISTINCT a, c) + GROUPING\n\n" | ||
| + "Supported scenarios:\n" | ||
| + "• Single COUNT(DISTINCT a, b) + GROUPING\n" | ||
| + "• Multiple COUNT(DISTINCT single_column) + " | ||
| + "GROUPING (e.g., COUNT(DISTINCT a), COUNT(DISTINCT b))"); | ||
| + "• A single DISTINCT argument group with GROUPING\n" | ||
| + "• Multiple DISTINCT argument groups when all DISTINCT aggregates support " | ||
| + "multi-distinct execution (e.g., COUNT(DISTINCT a), COUNT(DISTINCT b))"); | ||
| } | ||
| if (mustUseCte) { | ||
| return false; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -18,6 +18,7 @@ | ||
| package org.apache.doris.nereids.trees.expressions.functions.agg; | ||
| import org.apache.doris.catalog.FunctionSignature; | ||
| import org.apache.doris.nereids.exceptions.AnalysisException; | ||
| import org.apache.doris.nereids.trees.expressions.Expression; | ||
| import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; | ||
| import org.apache.doris.nereids.trees.expressions.literal.ArrayLiteral; | ||
| @@ -102,4 +103,18 @@ public List<FunctionSignature> getSignatures() { | ||
| public Expression resultForEmptyInput() { | ||
| return new ArrayLiteral(new ArrayList<>(), this.getDataType()); | ||
| } | ||
| @Override | ||
| public List<Expression> getDistinctArguments() { | ||
| return distinct ? ImmutableList.of(getArgument(0)) : ImmutableList.of(); | ||
morrySnow marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| @Override | ||
| public void checkLegalityBeforeTypeCoercion() { | ||
| if (arity() == 2 && !getArgument(1).isConstant()) { | ||
| throw new AnalysisException( | ||
| "collect_list requires second parameter must be a constant: " | ||
| + this.toSql()); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -53,7 +53,7 @@ public MapAgg(Expression arg0, Expression arg1) { | ||
| /** | ||
| * constructor with 2 arguments. | ||
| */ | ||
| private MapAgg(boolean distinct, Expression arg0, Expression arg1) { | ||
| public MapAgg(boolean distinct, Expression arg0, Expression arg1) { | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Add DISTINCT coverage for the v1 registration This makes the boolean constructor discoverable by the binder, but every added | ||
| super("map_agg_v1", distinct, arg0, arg1); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.