You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2024/06/07 11:14:57 UTC
(doris) branch 2.1_35899 created (now b6276fa87d1)
This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a change to branch 2.1_35899
in repository https://gitbox.apache.org/repos/asf/doris.git
at b6276fa87d1 [fix](Nereids) not do distinct when aggregate with distinct project
This branch includes the following new commits:
new b6276fa87d1 [fix](Nereids) not do distinct when aggregate with distinct project
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
(doris) 01/01: [fix](Nereids) not do distinct when aggregate with distinct project
Posted by mo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch 2.1_35899
in repository https://gitbox.apache.org/repos/asf/doris.git
commit b6276fa87d1ee389bae7a1f72ee51375aa4f3e1c
Author: morrySnow <10...@users.noreply.github.com>
AuthorDate: Thu Jun 6 16:55:40 2024 +0800
[fix](Nereids) not do distinct when aggregate with distinct project
pick from master #35899
This PR
1. support distinct project + aggregate:
for sql like: SELECT DISTINCT c1 FROM t GROUP BY c1, c2
should do distinct c1 after group by c1 and c2.
the plan should be:
Project(isDistinct=true, [c1])
+-- Aggregate(groupByKey=[c1, c2], output=[c1])
but we lost distinct project with:
Aggregate(groupByKey=[c1, c2], output=[c1])
the result is wrong because c1 do not DISTINCT at all
2. fix repeat column pruning bug prune grouping set columns
for sql like:
select a1, sum(sid)
from
(
select a + 1 as a1, b + 1 as b1, sum(id) as sid
from t
group by
grouping sets(
( a + 1, b ),
(b + 1),
(b + 2),
(b + 3)
)
) t
group by a1;
exception will be raised:
Caused by: java.util.NoSuchElementException: No value present
at java.util.Optional.get(Optional.java:143) ~[?:?]
at org.apache.doris.nereids.trees.plans.algebra.Repeat.getSortedVirtualSlots(Repeat.java:101) ~[classes/:?]
at org.apache.doris.nereids.glue.translator.PhysicalPlanTranslator.visitPhysicalRepeat(PhysicalPlanTranslator.java:2212) ~[classes/:?]
that because virtual column COL_GROUPING_ID be prune by mistake
---
.../glue/translator/PhysicalPlanTranslator.java | 2 +-
.../doris/nereids/parser/LogicalPlanBuilder.java | 9 ++-
.../org/apache/doris/nereids/rules/RuleType.java | 2 +
.../nereids/rules/analysis/BindExpression.java | 13 +++-
.../nereids/rules/analysis/FillUpMissingSlots.java | 64 ++++++++++++++++-
.../nereids/rules/analysis/NormalizeRepeat.java | 4 +-
.../rules/exploration/join/OuterJoinLAsscom.java | 2 +-
.../nereids/rules/rewrite/AdjustPreAggStatus.java | 2 +-
.../nereids/rules/rewrite/MergeAggregate.java | 2 +-
.../mv/SelectMaterializedIndexWithAggregate.java | 2 +-
.../org/apache/doris/nereids/trees/TreeNode.java | 4 +-
.../functions/ComputeSignatureHelper.java | 1 -
.../expressions/functions/udf/AliasUdfBuilder.java | 5 +-
.../doris/nereids/trees/plans/algebra/Repeat.java | 32 +++++++++
.../trees/plans/commands/DeleteFromCommand.java | 8 +--
.../insert/BatchInsertIntoTableCommand.java | 7 +-
.../commands/insert/InsertIntoTableCommand.java | 3 +-
.../insert/InsertOverwriteTableCommand.java | 3 +-
.../trees/plans/physical/PhysicalRepeat.java | 7 ++
.../apache/doris/nereids/util/ExpressionUtils.java | 5 +-
.../org/apache/doris/nereids/util/PlanUtils.java | 6 +-
.../rules/rewrite/mv/SelectMvIndexTest.java | 4 +-
.../aggregate/agg_with_distinct_project.out | 30 ++++++++
.../aggregate/agg_with_distinct_project.groovy | 80 ++++++++++++++++++++++
24 files changed, 257 insertions(+), 40 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
index e58e6c697c1..0d23e0a7874 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
@@ -948,7 +948,7 @@ public class PhysicalPlanTranslator extends DefaultPlanVisitor<PlanFragment, Pla
List<AggregateExpression> aggregateExpressionList = outputExpressions.stream()
.filter(o -> o.anyMatch(AggregateExpression.class::isInstance))
.peek(o -> aggFunctionOutput.add(o.toSlot()))
- .map(o -> o.<Set<AggregateExpression>>collect(AggregateExpression.class::isInstance))
+ .map(o -> o.<AggregateExpression>collect(AggregateExpression.class::isInstance))
.flatMap(Set::stream)
.collect(Collectors.toList());
ArrayList<FunctionCallExpr> execAggregateFunctions = aggregateExpressionList.stream()
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
index ea62df6dc18..a499e0e00ce 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
@@ -3110,10 +3110,15 @@ public class LogicalPlanBuilder extends DorisParserBaseVisitor<Object> {
}
private LogicalPlan withProjection(LogicalPlan input, SelectColumnClauseContext selectCtx,
- Optional<AggClauseContext> aggCtx, boolean isDistinct) {
+ Optional<AggClauseContext> aggCtx, boolean isDistinct) {
return ParserUtils.withOrigin(selectCtx, () -> {
if (aggCtx.isPresent()) {
- return input;
+ if (isDistinct) {
+ return new LogicalProject<>(ImmutableList.of(new UnboundStar(ImmutableList.of())),
+ Collections.emptyList(), isDistinct, input);
+ } else {
+ return input;
+ }
} else {
if (selectCtx.EXCEPT() != null) {
List<NamedExpression> expressions = getNamedExpressions(selectCtx.namedExpressionSeq());
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
index 45439e4cd51..6ed671a3334 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
@@ -60,6 +60,8 @@ public enum RuleType {
FILL_UP_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
FILL_UP_HAVING_PROJECT(RuleTypeClass.REWRITE),
FILL_UP_SORT_AGGREGATE(RuleTypeClass.REWRITE),
+ FILL_UP_SORT_AGGREGATE_AGGREGATE(RuleTypeClass.REWRITE),
+ FILL_UP_SORT_AGGREGATE_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
FILL_UP_SORT_HAVING_PROJECT(RuleTypeClass.REWRITE),
FILL_UP_SORT_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
FILL_UP_SORT_PROJECT(RuleTypeClass.REWRITE),
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
index 34af516c45b..c1080adf3b7 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
@@ -740,16 +740,25 @@ public class BindExpression implements AnalysisRuleFactory {
}
private Plan bindSortWithoutSetOperation(MatchingContext<LogicalSort<Plan>> ctx) {
+ CascadesContext cascadesContext = ctx.cascadesContext;
LogicalSort<Plan> sort = ctx.root;
Plan input = sort.child();
-
List<Slot> childOutput = input.getOutput();
+ // we should skip distinct project to bind slot in LogicalSort;
+ // check input.child(0) to avoid process SELECT DISTINCT a FROM t ORDER BY b by mistake
+ // NOTICE: SELECT a FROM (SELECT sum(a) AS a FROM t GROUP BY b) v ORDER BY b will not raise error result
+ // because input.child(0) is LogicalSubqueryAlias
+ if (input instanceof LogicalProject && ((LogicalProject<?>) input).isDistinct()
+ && (input.child(0) instanceof LogicalHaving
+ || input.child(0) instanceof LogicalAggregate
+ || input.child(0) instanceof LogicalRepeat)) {
+ input = input.child(0);
+ }
// we should skip LogicalHaving to bind slot in LogicalSort;
if (input instanceof LogicalHaving) {
input = input.child(0);
}
- CascadesContext cascadesContext = ctx.cascadesContext;
// 1. We should deduplicate the slots, otherwise the binding process will fail due to the
// ambiguous slots exist.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
index 1cab3614302..f78beb130e5 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
@@ -39,6 +39,7 @@ import org.apache.doris.nereids.util.ExpressionUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import java.util.List;
@@ -76,6 +77,22 @@ public class FillUpMissingSlots implements AnalysisRuleFactory {
sort.withChildren(new LogicalProject<>(projects, project.child())));
})
),
+ RuleType.FILL_UP_SORT_AGGREGATE_HAVING_AGGREGATE.build(
+ logicalSort(
+ aggregate(logicalHaving(aggregate()))
+ .when(a -> a.getOutputExpressions().stream().allMatch(SlotReference.class::isInstance))
+ ).when(this::checkSort)
+ .then(sort -> processDistinctProjectWithAggregate(sort, sort.child(), sort.child().child().child()))
+ ),
+ // ATTN: process aggregate with distinct project, must run this rule before FILL_UP_SORT_AGGREGATE
+ // because this pattern will always fail in FILL_UP_SORT_AGGREGATE
+ RuleType.FILL_UP_SORT_AGGREGATE_AGGREGATE.build(
+ logicalSort(
+ aggregate(aggregate())
+ .when(a -> a.getOutputExpressions().stream().allMatch(SlotReference.class::isInstance))
+ ).when(this::checkSort)
+ .then(sort -> processDistinctProjectWithAggregate(sort, sort.child(), sort.child().child()))
+ ),
RuleType.FILL_UP_SORT_AGGREGATE.build(
logicalSort(aggregate())
.when(this::checkSort)
@@ -334,7 +351,7 @@ public class FillUpMissingSlots implements AnalysisRuleFactory {
}
interface PlanGenerator {
- Plan apply(Resolver resolver, Aggregate aggregate);
+ Plan apply(Resolver resolver, Aggregate<?> aggregate);
}
private Plan createPlan(Resolver resolver, Aggregate<? extends Plan> aggregate, PlanGenerator planGenerator) {
@@ -371,4 +388,49 @@ public class FillUpMissingSlots implements AnalysisRuleFactory {
}
return false;
}
+
+ /**
+ * for sql like SELECT DISTINCT a FROM t GROUP BY a HAVING b > 0 ORDER BY a.
+ * there order by need to bind with bottom aggregate's output and bottom aggregate's child's output.
+ * this function used to fill up missing slot for these situations correctly.
+ *
+ * @param sort top sort
+ * @param upperAggregate upper aggregate used to check slot in order by should be in select list
+ * @param bottomAggregate bottom aggregate used to bind with its and its child's output
+ *
+ * @return filled up plan
+ */
+ private Plan processDistinctProjectWithAggregate(LogicalSort<?> sort,
+ Aggregate<?> upperAggregate, Aggregate<Plan> bottomAggregate) {
+ Resolver resolver = new Resolver(bottomAggregate);
+ sort.getExpressions().forEach(resolver::resolve);
+ return createPlan(resolver, bottomAggregate, (r, a) -> {
+ List<OrderKey> newOrderKeys = sort.getOrderKeys().stream()
+ .map(ok -> new OrderKey(
+ ExpressionUtils.replace(ok.getExpr(), r.getSubstitution()),
+ ok.isAsc(),
+ ok.isNullFirst()))
+ .collect(ImmutableList.toImmutableList());
+ boolean sortNotChanged = newOrderKeys.equals(sort.getOrderKeys());
+ boolean aggNotChanged = a.equals(bottomAggregate);
+ if (sortNotChanged && aggNotChanged) {
+ return null;
+ }
+ if (aggNotChanged) {
+ // since sort expr must in select list, we should not change agg at all.
+ return new LogicalSort<>(newOrderKeys, sort.child());
+ } else {
+ Set<NamedExpression> upperAggOutputs = Sets.newHashSet(upperAggregate.getOutputExpressions());
+ for (int i = 0; i < newOrderKeys.size(); i++) {
+ OrderKey orderKey = newOrderKeys.get(i);
+ Expression expression = orderKey.getExpr();
+ if (!upperAggOutputs.containsAll(expression.getInputSlots())) {
+ throw new AnalysisException(sort.getOrderKeys().get(i).getExpr().toSql()
+ + " of ORDER BY clause is not in SELECT list");
+ }
+ }
+ throw new AnalysisException("Expression of ORDER BY clause is not in SELECT list");
+ }
+ });
+ }
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java
index 2d39852dd18..c355effcffc 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java
@@ -372,14 +372,14 @@ public class NormalizeRepeat extends OneAnalysisRuleFactory {
CollectNonWindowedAggFuncs.collect(aggregate.getOutputExpressions());
ImmutableSet.Builder<Slot> aggUsedSlotBuilder = ImmutableSet.builder();
for (AggregateFunction function : aggregateFunctions) {
- aggUsedSlotBuilder.addAll(function.<Set<SlotReference>>collect(SlotReference.class::isInstance));
+ aggUsedSlotBuilder.addAll(function.<SlotReference>collect(SlotReference.class::isInstance));
}
ImmutableSet<Slot> aggUsedSlots = aggUsedSlotBuilder.build();
ImmutableSet.Builder<Slot> groupingSetsUsedSlotBuilder = ImmutableSet.builder();
for (List<Expression> groupingSet : repeat.getGroupingSets()) {
for (Expression expr : groupingSet) {
- groupingSetsUsedSlotBuilder.addAll(expr.<Set<SlotReference>>collect(SlotReference.class::isInstance));
+ groupingSetsUsedSlotBuilder.addAll(expr.<SlotReference>collect(SlotReference.class::isInstance));
}
}
ImmutableSet<Slot> groupingSetsUsedSlot = groupingSetsUsedSlotBuilder.build();
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/OuterJoinLAsscom.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/OuterJoinLAsscom.java
index 40ef9b96229..f10daeadcd9 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/OuterJoinLAsscom.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/OuterJoinLAsscom.java
@@ -96,7 +96,7 @@ public class OuterJoinLAsscom extends OneExplorationRuleFactory {
topJoin.getHashJoinConjuncts().stream(),
topJoin.getOtherJoinConjuncts().stream())
.allMatch(expr -> {
- Set<ExprId> usedExprIdSet = expr.<Set<SlotReference>>collect(SlotReference.class::isInstance)
+ Set<ExprId> usedExprIdSet = expr.<SlotReference>collect(SlotReference.class::isInstance)
.stream()
.map(SlotReference::getExprId)
.collect(Collectors.toSet());
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustPreAggStatus.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustPreAggStatus.java
index 8b90e4cdedc..495a06870f5 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustPreAggStatus.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AdjustPreAggStatus.java
@@ -383,7 +383,7 @@ public class AdjustPreAggStatus implements RewriteRuleFactory {
project.map(Project::getAliasToProducer);
return agg.getOutputExpressions().stream()
// extract aggregate functions.
- .flatMap(e -> e.<Set<AggregateFunction>>collect(AggregateFunction.class::isInstance)
+ .flatMap(e -> e.<AggregateFunction>collect(AggregateFunction.class::isInstance)
.stream())
// replace aggregate function's input slot by its producing expression.
.map(expr -> slotToProducerOpt
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java
index 889adfb69f5..8b4a724d073 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java
@@ -150,7 +150,7 @@ public class MergeAggregate implements RewriteRuleFactory {
});
}
- boolean commonCheck(LogicalAggregate<? extends Plan> outerAgg, LogicalAggregate<Plan> innerAgg,
+ private boolean commonCheck(LogicalAggregate<? extends Plan> outerAgg, LogicalAggregate<Plan> innerAgg,
boolean sameGroupBy, Optional<LogicalProject> projectOptional) {
innerAggExprIdToAggFunc = innerAgg.getOutputExpressions().stream()
.filter(expr -> (expr instanceof Alias) && (expr.child(0) instanceof AggregateFunction))
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
index cb03a0c5840..f710372d6eb 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMaterializedIndexWithAggregate.java
@@ -674,7 +674,7 @@ public class SelectMaterializedIndexWithAggregate extends AbstractSelectMaterial
Optional<Map<Slot, Expression>> slotToProducerOpt = project.map(Project::getAliasToProducer);
return agg.getOutputExpressions().stream()
// extract aggregate functions.
- .flatMap(e -> e.<Set<AggregateFunction>>collect(AggregateFunction.class::isInstance).stream())
+ .flatMap(e -> e.<AggregateFunction>collect(AggregateFunction.class::isInstance).stream())
// replace aggregate function's input slot by its producing expression.
.map(expr -> slotToProducerOpt.map(slotToExpressions
-> (AggregateFunction) ExpressionUtils.replace(expr, slotToExpressions))
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TreeNode.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TreeNode.java
index 6d1a298eb79..a4bfab08890 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TreeNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TreeNode.java
@@ -232,14 +232,14 @@ public interface TreeNode<NODE_TYPE extends TreeNode<NODE_TYPE>> {
/**
* Collect the nodes that satisfied the predicate.
*/
- default <T> T collect(Predicate<TreeNode<NODE_TYPE>> predicate) {
+ default <T> Set<T> collect(Predicate<TreeNode<NODE_TYPE>> predicate) {
ImmutableSet.Builder<TreeNode<NODE_TYPE>> result = ImmutableSet.builder();
foreach(node -> {
if (predicate.test(node)) {
result.add(node);
}
});
- return (T) result.build();
+ return (Set<T>) result.build();
}
/**
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java
index 2cdbe43c12e..166f1c9db7f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java
@@ -39,7 +39,6 @@ import org.apache.doris.nereids.util.TypeCoercionUtils;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableList.Builder;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/udf/AliasUdfBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/udf/AliasUdfBuilder.java
index 1f15b7e6049..9ddb8ea25e5 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/udf/AliasUdfBuilder.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/udf/AliasUdfBuilder.java
@@ -33,7 +33,6 @@ import com.google.common.collect.Maps;
import java.util.List;
import java.util.Map;
import java.util.Optional;
-import java.util.Set;
import java.util.stream.Collectors;
/**
@@ -85,8 +84,8 @@ public class AliasUdfBuilder extends UdfBuilder {
// replace the placeholder slot to the input expressions.
// adjust input, parameter and replaceMap to be corresponding.
- Map<String, SlotReference> slots = ((Set<SlotReference>) boundFunction
- .collect(SlotReference.class::isInstance))
+ Map<String, SlotReference> slots = (boundFunction
+ .<SlotReference>collect(SlotReference.class::isInstance))
.stream().collect(Collectors.toMap(SlotReference::getName, k -> k, (v1, v2) -> v2));
Map<SlotReference, Expression> replaceMap = Maps.newHashMap();
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java
index e729f2a7cb3..8925e597850 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java
@@ -58,6 +58,38 @@ public interface Repeat<CHILD_PLAN extends Plan> extends Aggregate<CHILD_PLAN> {
return ExpressionUtils.flatExpressions(getGroupingSets());
}
+ @Override
+ default Aggregate<CHILD_PLAN> pruneOutputs(List<NamedExpression> prunedOutputs) {
+ // just output reserved outputs and COL_GROUPING_ID for repeat correctly.
+ ImmutableList.Builder<NamedExpression> outputBuilder
+ = ImmutableList.builderWithExpectedSize(prunedOutputs.size() + 1);
+ outputBuilder.addAll(prunedOutputs);
+ for (NamedExpression output : getOutputExpressions()) {
+ Set<VirtualSlotReference> v = output.collect(VirtualSlotReference.class::isInstance);
+ if (v.stream().anyMatch(slot -> slot.getName().equals(COL_GROUPING_ID))) {
+ outputBuilder.add(output);
+ }
+ }
+ // prune groupingSets, if parent operator do not need some exprs in grouping sets, we removed it.
+ // this could not lead to wrong result because be repeat other columns by normal.
+ ImmutableList.Builder<List<Expression>> groupingSetsBuilder
+ = ImmutableList.builderWithExpectedSize(getGroupingSets().size());
+ for (List<Expression> groupingSet : getGroupingSets()) {
+ ImmutableList.Builder<Expression> groupingSetBuilder
+ = ImmutableList.builderWithExpectedSize(groupingSet.size());
+ for (Expression expr : groupingSet) {
+ if (prunedOutputs.contains(expr)) {
+ groupingSetBuilder.add(expr);
+ }
+ }
+ groupingSetsBuilder.add(groupingSetBuilder.build());
+ }
+ return withGroupSetsAndOutput(groupingSetsBuilder.build(), outputBuilder.build());
+ }
+
+ Repeat<CHILD_PLAN> withGroupSetsAndOutput(List<List<Expression>> groupingSets,
+ List<NamedExpression> outputExpressions);
+
static VirtualSlotReference generateVirtualGroupingIdSlot() {
return new VirtualSlotReference(COL_GROUPING_ID, BigIntType.INSTANCE, Optional.empty(),
GroupingSetShapes::computeVirtualGroupingIdValue);
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java
index 62feee1c43f..6563d815382 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java
@@ -109,13 +109,13 @@ public class DeleteFromCommand extends Command implements ForwardWithSync {
return;
}
Optional<PhysicalFilter<?>> optFilter = (planner.getPhysicalPlan()
- .<Set<PhysicalFilter<?>>>collect(PhysicalFilter.class::isInstance)).stream()
+ .<PhysicalFilter<?>>collect(PhysicalFilter.class::isInstance)).stream()
.findAny();
Optional<PhysicalOlapScan> optScan = (planner.getPhysicalPlan()
- .<Set<PhysicalOlapScan>>collect(PhysicalOlapScan.class::isInstance)).stream()
+ .<PhysicalOlapScan>collect(PhysicalOlapScan.class::isInstance)).stream()
.findAny();
Optional<UnboundRelation> optRelation = (logicalQuery
- .<Set<UnboundRelation>>collect(UnboundRelation.class::isInstance)).stream()
+ .<UnboundRelation>collect(UnboundRelation.class::isInstance)).stream()
.findAny();
Preconditions.checkArgument(optFilter.isPresent(), "delete command must contain filter");
Preconditions.checkArgument(optScan.isPresent(), "delete command could be only used on olap table");
@@ -141,7 +141,7 @@ public class DeleteFromCommand extends Command implements ForwardWithSync {
Plan plan = planner.getPhysicalPlan();
checkSubQuery(plan);
for (Expression conjunct : filter.getConjuncts()) {
- conjunct.<Set<SlotReference>>collect(SlotReference.class::isInstance)
+ conjunct.<SlotReference>collect(SlotReference.class::isInstance)
.forEach(s -> checkColumn(columns, s, olapTable));
checkPredicate(conjunct);
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java
index 4399cd57db4..4b7afb1f6a8 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java
@@ -53,7 +53,6 @@ import org.apache.logging.log4j.Logger;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
-import java.util.Set;
import java.util.stream.Collectors;
/**
@@ -111,7 +110,7 @@ public class BatchInsertIntoTableCommand extends Command implements NoForward, E
}
Optional<TreeNode<?>> plan = planner.getPhysicalPlan()
- .<Set<TreeNode<?>>>collect(PhysicalOlapTableSink.class::isInstance).stream().findAny();
+ .<TreeNode<?>>collect(PhysicalOlapTableSink.class::isInstance).stream().findAny();
Preconditions.checkArgument(plan.isPresent(), "insert into command must contain OlapTableSinkNode");
sink = ((PhysicalOlapTableSink<?>) plan.get());
Table targetTable = sink.getTargetTable();
@@ -141,14 +140,14 @@ public class BatchInsertIntoTableCommand extends Command implements NoForward, E
}
Optional<PhysicalUnion> union = planner.getPhysicalPlan()
- .<Set<PhysicalUnion>>collect(PhysicalUnion.class::isInstance).stream().findAny();
+ .<PhysicalUnion>collect(PhysicalUnion.class::isInstance).stream().findAny();
if (union.isPresent()) {
InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(),
targetTable.getName(), targetSchema, union.get().getConstantExprsList());
return;
}
Optional<PhysicalOneRowRelation> oneRowRelation = planner.getPhysicalPlan()
- .<Set<PhysicalOneRowRelation>>collect(PhysicalOneRowRelation.class::isInstance).stream().findAny();
+ .<PhysicalOneRowRelation>collect(PhysicalOneRowRelation.class::isInstance).stream().findAny();
if (oneRowRelation.isPresent()) {
InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(),
targetTable.getName(), targetSchema, ImmutableList.of(oneRowRelation.get().getProjects()));
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java
index 4118cd1ddaa..a88b8cc2e05 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java
@@ -54,7 +54,6 @@ import org.apache.logging.log4j.Logger;
import java.util.Objects;
import java.util.Optional;
-import java.util.Set;
/**
* insert into select command implementation
@@ -152,7 +151,7 @@ public class InsertIntoTableCommand extends Command implements ForwardWithSync,
ctx.getMysqlChannel().reset();
}
Optional<PhysicalSink<?>> plan = (planner.getPhysicalPlan()
- .<Set<PhysicalSink<?>>>collect(PhysicalSink.class::isInstance)).stream()
+ .<PhysicalSink<?>>collect(PhysicalSink.class::isInstance)).stream()
.findAny();
Preconditions.checkArgument(plan.isPresent(), "insert into command must contain target table");
PhysicalSink physicalSink = plan.get();
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertOverwriteTableCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertOverwriteTableCommand.java
index d9047dcfc7d..34d9c093718 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertOverwriteTableCommand.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertOverwriteTableCommand.java
@@ -60,7 +60,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
-import java.util.Set;
/**
* insert into select command implementation
@@ -122,7 +121,7 @@ public class InsertOverwriteTableCommand extends Command implements ForwardWithS
}
Optional<TreeNode<?>> plan = (planner.getPhysicalPlan()
- .<Set<TreeNode<?>>>collect(node -> node instanceof PhysicalTableSink)).stream().findAny();
+ .<TreeNode<?>>collect(node -> node instanceof PhysicalTableSink)).stream().findAny();
Preconditions.checkArgument(plan.isPresent(), "insert into command must contain OlapTableSinkNode");
PhysicalTableSink<?> physicalTableSink = ((PhysicalTableSink<?>) plan.get());
TableIf targetTable = physicalTableSink.getTargetTable();
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java
index 3cb6f730069..1ef2ff77d5a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java
@@ -178,6 +178,13 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan> extends PhysicalUnary<CHILD
getLogicalProperties(), physicalProperties, statistics, child());
}
+ @Override
+ public PhysicalRepeat<CHILD_TYPE> withGroupSetsAndOutput(List<List<Expression>> groupingSets,
+ List<NamedExpression> outputExpressionList) {
+ return new PhysicalRepeat<>(groupingSets, outputExpressionList, Optional.empty(),
+ getLogicalProperties(), physicalProperties, statistics, child());
+ }
+
@Override
public PhysicalRepeat<CHILD_TYPE> resetLogicalProperties() {
return new PhysicalRepeat<>(groupingSets, outputExpressions, groupExpression,
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
index a6a4d999a92..b19d4b096e2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
@@ -65,6 +65,7 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
import java.util.Collection;
@@ -522,9 +523,7 @@ public class ExpressionUtils {
ImmutableList<Literal> literals =
ImmutableList.of(new NullLiteral(BooleanType.INSTANCE), BooleanLiteral.FALSE);
List<MarkJoinSlotReference> markJoinSlotReferenceList =
- ((Set<MarkJoinSlotReference>) predicate
- .collect(MarkJoinSlotReference.class::isInstance)).stream()
- .collect(Collectors.toList());
+ new ArrayList<>((predicate.collect(MarkJoinSlotReference.class::isInstance)));
int markSlotSize = markJoinSlotReferenceList.size();
int maxMarkSlotCount = 4;
// if the conjunct has mark slot, and maximum 4 mark slots(for performance)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java
index 9c5e6b318e8..7cfa7b7709e 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java
@@ -40,7 +40,6 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import java.util.Collection;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -140,10 +139,7 @@ public class PlanUtils {
}
public static Set<LogicalCatalogRelation> getLogicalScanFromRootPlan(LogicalPlan rootPlan) {
- Set<LogicalCatalogRelation> tableSet = new HashSet<>();
- tableSet.addAll((Collection<? extends LogicalCatalogRelation>) rootPlan
- .collect(LogicalCatalogRelation.class::isInstance));
- return tableSet;
+ return rootPlan.collect(LogicalCatalogRelation.class::isInstance);
}
/**
diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java
index 83b969d9f12..bc8df2ab0f7 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/mv/SelectMvIndexTest.java
@@ -1224,7 +1224,7 @@ class SelectMvIndexTest extends BaseMaterializedIndexSelectTest implements MemoP
private void assertOneAggFuncType(LogicalAggregate<? extends Plan> agg, Class<?> aggFuncType) {
Set<AggregateFunction> aggFuncs = agg.getOutputExpressions()
.stream()
- .flatMap(e -> e.<Set<AggregateFunction>>collect(AggregateFunction.class::isInstance)
+ .flatMap(e -> e.<AggregateFunction>collect(AggregateFunction.class::isInstance)
.stream())
.collect(Collectors.toSet());
Assertions.assertEquals(1, aggFuncs.size());
@@ -1239,7 +1239,7 @@ class SelectMvIndexTest extends BaseMaterializedIndexSelectTest implements MemoP
Assertions.assertEquals(2, scans.size());
ScanNode scanNode0 = scans.get(0);
- Assertions.assertTrue(scanNode0 instanceof OlapScanNode);
+ Assertions.assertInstanceOf(OlapScanNode.class, scanNode0);
OlapScanNode scan0 = (OlapScanNode) scanNode0;
Assertions.assertTrue(scan0.isPreAggregation());
Assertions.assertEquals(firstTableIndexName, scan0.getSelectedIndexName());
diff --git a/regression-test/data/nereids_p0/aggregate/agg_with_distinct_project.out b/regression-test/data/nereids_p0/aggregate/agg_with_distinct_project.out
new file mode 100644
index 00000000000..ac5a1851bcb
--- /dev/null
+++ b/regression-test/data/nereids_p0/aggregate/agg_with_distinct_project.out
@@ -0,0 +1,30 @@
+-- This file is automatically generated. You should know what you did if you want to edit this
+-- !base_case --
+0
+81
+
+-- !with_order --
+1
+82
+
+-- !with_having --
+1
+82
+
+-- !with_having_with_order --
+1
+82
+
+-- !with_order_with_grouping_sets --
+\N
+1
+82
+
+-- !with_having_with_grouping_sets --
+1
+82
+
+-- !with_having_with_order_with_grouping_sets --
+1
+82
+
diff --git a/regression-test/suites/nereids_p0/aggregate/agg_with_distinct_project.groovy b/regression-test/suites/nereids_p0/aggregate/agg_with_distinct_project.groovy
new file mode 100644
index 00000000000..fddd47884e1
--- /dev/null
+++ b/regression-test/suites/nereids_p0/aggregate/agg_with_distinct_project.groovy
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+suite("agg_with_distinct_project") {
+ sql "DROP TABLE IF EXISTS agg_with_distinct_project;"
+ sql """
+ CREATE TABLE agg_with_distinct_project (
+ id int NOT NULL,
+ a int DEFAULT NULL,
+ b int DEFAULT NULL
+ )
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1"
+ );
+ """
+
+ sql """INSERT INTO agg_with_distinct_project VALUES(83,0,38),(26,0,79),(43,81,24)"""
+
+ order_qt_base_case """
+ SELECT DISTINCT a as c1 FROM agg_with_distinct_project GROUP BY b, a;
+ """
+
+ qt_with_order """
+ select distinct a + 1 from agg_with_distinct_project group by a + 1, b order by a + 1;
+ """
+
+ order_qt_with_having """
+ select distinct a + 1 from agg_with_distinct_project group by a + 1, b having b > 1;
+ """
+
+ qt_with_having_with_order """
+ select distinct a + 1 from agg_with_distinct_project group by a + 1, b having b > 1 order by a + 1;
+ """
+
+ qt_with_order_with_grouping_sets """
+ select distinct a + 1 from agg_with_distinct_project group by grouping sets(( a + 1, b ), (b + 1)) order by a + 1;
+ """
+
+ order_qt_with_having_with_grouping_sets """
+ select distinct a + 1 from agg_with_distinct_project group by grouping sets(( a + 1, b ), (b + 1)) having b > 1;
+ """
+
+ qt_with_having_with_order_with_grouping_sets """
+ select distinct a + 1 from agg_with_distinct_project group by grouping sets(( a + 1, b ), (b + 1)) having b > 1 order by a + 1;
+ """
+
+ // order by column not in select list
+ test {
+ sql """
+ select distinct a + 1 from agg_with_distinct_project group by a + 1, b order by b;
+ """
+ exception "b of ORDER BY clause is not in SELECT list"
+ }
+
+ // order by column not in select list
+ test {
+ sql """
+ select distinct a + 1 from agg_with_distinct_project group by grouping sets(( a + 1, b ), (b + 1)) order by b;
+ """
+ exception "b of ORDER BY clause is not in SELECT list"
+ }
+
+ sql "DROP TABLE IF EXISTS agg_with_distinct_project;"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org