Skip to content

Commit

Permalink
[fix](Nereids) record wrong best plan properties (apache#23973)
Browse files Browse the repository at this point in the history 
when output meet order by not meet distribution. we use a trick way to
do enforce by set current output to any. but when we do enforce later,
we still use the old output. So when we do choose best plan, we could
not find the older output's plan, since we have replace it by any.
For example:

```
  lowest Plan(cost, properties, plan, childrenRequires)

    18.0 ANY
     id:138#4 cost=0 [0/0/0/] estRows=4 children=[@0 ] (plan=PhysicalWindow[139]@4 ( windowFrameGroup=(Funcs=[row_number() WindowSpec(PARTITION BY b#1, a#0 ROWS BETWEEN UNBOUNDED_PRECEDING AND CURRENT_ROW) AS `r1`#2], PartitionKeys=[b#1, a#0], OrderKeys=[], WindowFrame=WindowFrame(ROWS, UNBOUNDED_PRECEDING, CURRENT_ROW)), requiredProperties=[DistributionSpecHash ( orderedShuffledColumns=[1, 0], shuffleType=REQUIRE, tableId=-1, selectedIndexId=-1, partitionIds=[], equivalenceExprIds=[[1], [0]], exprIdToEquivalenceSet={0=1, 1=0} ) Order: ([b#1 asc, a#0 asc])], stats=null ))
     [DistributionSpecHash ( orderedShuffledColumns=[0], shuffleType=NATURAL, tableId=3547296, selectedIndexId=3547297, partitionIds=[3547295], equivalenceExprIds=[[0]], exprIdToEquivalenceSet={0=0} ) Order: ([b#1 asc, a#0 asc])]

    32.01171875 DistributionSpecHash ( orderedShuffledColumns=[1], shuffleType=REQUIRE, tableId=-1, selectedIndexId=-1, partitionIds=[], equivalenceExprIds=[[1]], exprIdToEquivalenceSet={1=0} ) Order: ([b#1 asc])
     id:161#4 cost=14 [4/4/4/] estRows=4 children=[@4 ] (plan=PhysicalQuickSort[162]@4 ( orderKeys=[b#1 asc], phase=LOCAL_SORT, stats=null ))
     [DistributionSpecHash ( orderedShuffledColumns=[0], shuffleType=NATURAL, tableId=3547296, selectedIndexId=3547297, partitionIds=[3547295], equivalenceExprIds=[[0]], exprIdToEquivalenceSet={0=0} ) Order: ([b#1 asc, a#0 asc])]

    32.01171875 DistributionSpecHash ( orderedShuffledColumns=[1], shuffleType=EXECUTION_BUCKETED, tableId=-1, selectedIndexId=-1, partitionIds=[], equivalenceExprIds=[[1]], exprIdToEquivalenceSet={1=0} ) Order: ([b#1 asc])
     id:161#4 cost=14 [4/4/4/] estRows=4 children=[@4 ] (plan=PhysicalQuickSort[162]@4 ( orderKeys=[b#1 asc], phase=LOCAL_SORT, stats=null ))
     [DistributionSpecHash ( orderedShuffledColumns=[1], shuffleType=EXECUTION_BUCKETED, tableId=-1, selectedIndexId=-1, partitionIds=[], equivalenceExprIds=[[1]], exprIdToEquivalenceSet={1=0} ) Order: ([])]

    18.01171875 DistributionSpecHash ( orderedShuffledColumns=[1], shuffleType=EXECUTION_BUCKETED, tableId=-1, selectedIndexId=-1, partitionIds=[], equivalenceExprIds=[[1]], exprIdToEquivalenceSet={1=0} ) Order: ([])
     id:157#4 cost=0 [0/0/0/] estRows=4 children=[@4 ] (plan=PhysicalDistribute[158]@4 ( distributionSpec=DistributionSpecHash ( orderedShuffledColumns=[1], shuffleType=EXECUTION_BUCKETED, tableId=-1, selectedIndexId=-1, partitionIds=[], equivalenceExprIds=[[1]], exprIdToEquivalenceSet={1=0} ), stats=null ))
     [DistributionSpecHash ( orderedShuffledColumns=[0], shuffleType=NATURAL, tableId=3547296, selectedIndexId=3547297, partitionIds=[3547295], equivalenceExprIds=[[0]], exprIdToEquivalenceSet={0=0} ) Order: ([b#1 asc, a#0 asc])]
```

the last one require a natural shuffle type property from this group.
but this property already been removed when we do
enforceDistributionButMeetSort. So, such exception will be thrown

```
Caused by: org.apache.doris.nereids.exceptions.AnalysisException: Failed to choose best plan
    at org.apache.doris.nereids.NereidsPlanner.chooseBestPlan(NereidsPlanner.java:340) ~[classes/:?]
    at org.apache.doris.nereids.NereidsPlanner.chooseBestPlan(NereidsPlanner.java:323) ~[classes/:?]
    ... 18 more
Caused by: org.apache.doris.nereids.exceptions.AnalysisException: lowestCostPlans with physicalProperties(DistributionSpecHash ( orderedShuffledColumns=[0], shuffleType=NATURAL, tableId=3547296, selectedIndexId=3547297, partitionIds=[35
47295], equivalenceExprIds=[[0]], exprIdToEquivalenceSet={0=0} ) Order: ([b#1 asc, a#0 asc])) doesn't exist in root group
    at org.apache.doris.nereids.NereidsPlanner.lambda$chooseBestPlan$1(NereidsPlanner.java:318) ~[classes/:?]
    at java.util.Optional.orElseThrow(Optional.java:408) ~[?:?]
    at org.apache.doris.nereids.NereidsPlanner.chooseBestPlan(NereidsPlanner.java:317) ~[classes/:?]
    at org.apache.doris.nereids.NereidsPlanner.chooseBestPlan(NereidsPlanner.java:323) ~[classes/:?]
    ... 18 more
```
  • Loading branch information
@morrySnow
morrySnow authored Sep 7, 2023
1 parent f2ebe65 commit b2ca281
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 33 deletions.
19 changes: 5 additions & 14 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -335,20 +332,14 @@ private PhysicalPlan chooseBestPlan(Group rootGroup, PhysicalProperties physical
groupExpression.getOwnerGroup().getStatistics());
return physicalPlan;
} catch (Exception e) {
String memo = cascadesContext.getMemo().toString();
LOG.warn("Failed to choose best plan, memo structure:{}", memo, e);
throw new AnalysisException("Failed to choose best plan", e);
if (e instanceof AnalysisException && e.getMessage().contains("Failed to choose best plan")) {
throw e;
}
LOG.warn("Failed to choose best plan, memo structure:{}", cascadesContext.getMemo(), e);
throw new AnalysisException("Failed to choose best plan: " + e.getMessage(), e);
}
}

private ScheduledExecutorService runTimeoutExecutor() {
ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
Runnable task = () -> cascadesContext.setIsTimeout(true);
executor.schedule(task, 5, TimeUnit.SECONDS);

return executor;
}

/**
* getting hints explain string, which specified by enumerate and show in lists
* @param hintMap hint map recorded in statement context
Expand Down
17 changes: 16 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,14 +438,29 @@ public String toString() {
for (GroupExpression physicalExpression : physicalExpressions) {
str.append(" ").append(physicalExpression).append("\n");
}
str.append(" enforcers:\n");
str.append(" enforcers:\n");
for (GroupExpression enforcer : enforcers) {
str.append(" ").append(enforcer).append("\n");
}
if (chosenGroupExpressionId != -1) {
str.append(" chosen expression id: ").append(chosenGroupExpressionId).append("\n");
str.append(" chosen properties: ").append(chosenProperties).append("\n");
}
str.append(" stats").append("\n");
str.append(getStatistics() == null ? "" : getStatistics().detail(" "));
str.append(" lowest Plan(cost, properties, plan, childrenRequires)");
getAllProperties().forEach(
prop -> {
Optional<Pair<Cost, GroupExpression>> costAndGroupExpression = getLowestCostPlan(prop);
if (costAndGroupExpression.isPresent()) {
Cost cost = costAndGroupExpression.get().first;
GroupExpression child = costAndGroupExpression.get().second;
str.append("\n\n ").append(cost.getValue()).append(" ").append(prop)
.append("\n ").append(child).append("\n ")
.append(child.getInputPropertiesListOrEmpty(prop));
}
}
);
return str.toString();
}

Expand Down
18 changes: 1 addition & 17 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,23 +779,7 @@ public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("root:").append(getRoot()).append("\n");
for (Group group : groups.values()) {
builder.append("\n\n").append(group);
builder.append(" stats").append("\n");
builder.append(group.getStatistics().detail(" "));
builder.append(" lowest Plan(cost, properties, plan, childrenRequires)");
group.getAllProperties().forEach(
prop -> {
Optional<Pair<Cost, GroupExpression>> costAndGroupExpression = group.getLowestCostPlan(prop);
if (costAndGroupExpression.isPresent()) {
Cost cost = costAndGroupExpression.get().first;
GroupExpression child = costAndGroupExpression.get().second;
builder.append("\n\n " + cost.getValue() + " " + prop)
.append("\n ").append(child)
.append("\n " + child.getInputPropertiesListOrEmpty(prop));
}
}
);
builder.append("\n");
builder.append("\n\n").append(group).append("\n");
}
return builder.toString();
}
Expand Down
2 changes: 1 addition & 1 deletion ...ore/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private PhysicalProperties enforceDistributionButMeetSort(PhysicalProperties out
groupExpression.getOwnerGroup()
.replaceBestPlanProperty(
output, PhysicalProperties.ANY, groupExpression.getCostValueByProperties(output));
return enforceSortAndDistribution(output, request);
return enforceSortAndDistribution(PhysicalProperties.ANY, request);
}

private PhysicalProperties enforceGlobalSort(PhysicalProperties oldOutputProperty, PhysicalProperties required) {
Expand Down

0 comments on commit b2ca281

Please sign in to comment.