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 cf30ea914a [fix](Nereids) forbid gather sort with explict shuffle
(#22153)
cf30ea914a is described below
commit cf30ea914a8c06742f13f8ce95be1d0eef758ae3
Author: morrySnow <[email protected]>
AuthorDate: Mon Jul 24 19:45:18 2023 +0800
[fix](Nereids) forbid gather sort with explict shuffle (#22153)
gather sort with explict shuffle usually bad, forbid it
---
.../nereids/properties/ChildrenPropertiesRegulator.java | 17 ++++++++++++++++-
.../implementation/LogicalSortToPhysicalQuickSort.java | 8 ++++----
.../rules/implementation/LogicalTopNToPhysicalTopN.java | 11 ++++++-----
3 files changed, 26 insertions(+), 10 deletions(-)
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 54e4c4780a..05c8641486 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
@@ -31,6 +31,8 @@ import
org.apache.doris.nereids.trees.expressions.SlotReference;
import
org.apache.doris.nereids.trees.expressions.functions.agg.MultiDistinction;
import org.apache.doris.nereids.trees.plans.AggMode;
import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.SortPhase;
+import org.apache.doris.nereids.trees.plans.physical.AbstractPhysicalSort;
import org.apache.doris.nereids.trees.plans.physical.PhysicalDistribute;
import org.apache.doris.nereids.trees.plans.physical.PhysicalFilter;
import org.apache.doris.nereids.trees.plans.physical.PhysicalHashAggregate;
@@ -51,7 +53,9 @@ import java.util.Set;
import java.util.stream.Collectors;
/**
- * ensure child add enough distribute. update children properties if we do
regular
+ * ensure child add enough distribute. update children properties if we do
regular.
+ * NOTICE: all visitor should call visit(plan, context) at proper place
+ * to process must shuffle except project and filter
*/
public class ChildrenPropertiesRegulator extends PlanVisitor<Boolean, Void> {
@@ -364,6 +368,17 @@ public class ChildrenPropertiesRegulator extends
PlanVisitor<Boolean, Void> {
return true;
}
+ @Override
+ public Boolean visitAbstractPhysicalSort(AbstractPhysicalSort<? extends
Plan> sort, Void context) {
+ // process must shuffle
+ visit(sort, context);
+ if (sort.getSortPhase() == SortPhase.GATHER_SORT && sort.child()
instanceof PhysicalDistribute) {
+ // forbid gather sort need explicit shuffle
+ return false;
+ }
+ return true;
+ }
+
private boolean bothSideShuffleKeysAreSameOrder(
DistributionSpecHash notShuffleSideOutput, DistributionSpecHash
shuffleSideOutput,
DistributionSpecHash notShuffleSideRequired, DistributionSpecHash
shuffleSideRequired) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalSortToPhysicalQuickSort.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalSortToPhysicalQuickSort.java
index a8cb23a4f8..a90770e071 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalSortToPhysicalQuickSort.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalSortToPhysicalQuickSort.java
@@ -38,14 +38,14 @@ public class LogicalSortToPhysicalQuickSort extends
OneImplementationRuleFactory
.toRule(RuleType.LOGICAL_SORT_TO_PHYSICAL_QUICK_SORT_RULE);
}
- private List<PhysicalQuickSort<? extends Plan>> twoPhaseSort(LogicalSort
logicalSort) {
- PhysicalQuickSort localSort = new
PhysicalQuickSort(logicalSort.getOrderKeys(),
+ private List<PhysicalQuickSort<? extends Plan>> twoPhaseSort(LogicalSort<?
extends Plan> logicalSort) {
+ PhysicalQuickSort<Plan> localSort = new
PhysicalQuickSort<>(logicalSort.getOrderKeys(),
SortPhase.LOCAL_SORT, logicalSort.getLogicalProperties(),
logicalSort.child(0));
- PhysicalQuickSort twoPhaseSort = new PhysicalQuickSort<>(
+ PhysicalQuickSort<Plan> twoPhaseSort = new PhysicalQuickSort<>(
logicalSort.getOrderKeys(),
SortPhase.MERGE_SORT, logicalSort.getLogicalProperties(),
localSort);
- PhysicalQuickSort onePhaseSort = new PhysicalQuickSort<>(
+ PhysicalQuickSort<Plan> onePhaseSort = new PhysicalQuickSort<>(
logicalSort.getOrderKeys(),
SortPhase.GATHER_SORT, logicalSort.getLogicalProperties(),
localSort.child(0));
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalTopNToPhysicalTopN.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalTopNToPhysicalTopN.java
index bf675fe264..6a94fe79cf 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalTopNToPhysicalTopN.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalTopNToPhysicalTopN.java
@@ -44,14 +44,15 @@ public class LogicalTopNToPhysicalTopN extends
OneImplementationRuleFactory {
* gatherTopN(limit, off, require gather)
* mergeTopN(limit, off, require gather) -> localTopN(off+limit, 0,
require any)
*/
- private List<PhysicalTopN<? extends Plan>> twoPhaseSort(LogicalTopN
logicalTopN) {
- PhysicalTopN localSort = new PhysicalTopN(logicalTopN.getOrderKeys(),
+ private List<PhysicalTopN<? extends Plan>> twoPhaseSort(LogicalTopN<?
extends Plan> logicalTopN) {
+ PhysicalTopN<Plan> localSort = new
PhysicalTopN<>(logicalTopN.getOrderKeys(),
logicalTopN.getLimit() + logicalTopN.getOffset(), 0,
SortPhase.LOCAL_SORT,
logicalTopN.getLogicalProperties(), logicalTopN.child(0));
- PhysicalTopN twoPhaseSort = new
PhysicalTopN<>(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
+ PhysicalTopN<Plan> twoPhaseSort = new
PhysicalTopN<>(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
logicalTopN.getOffset(), SortPhase.MERGE_SORT,
logicalTopN.getLogicalProperties(), localSort);
- PhysicalTopN onePhaseSort = new
PhysicalTopN<>(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
- logicalTopN.getOffset(), SortPhase.GATHER_SORT,
logicalTopN.getLogicalProperties(), localSort.child(0));
+ PhysicalTopN<Plan> onePhaseSort = new
PhysicalTopN<>(logicalTopN.getOrderKeys(), logicalTopN.getLimit(),
+ logicalTopN.getOffset(), SortPhase.GATHER_SORT,
+ logicalTopN.getLogicalProperties(), localSort.child(0));
return Lists.newArrayList(twoPhaseSort, onePhaseSort);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
