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 b2ca281395 [fix](Nereids) record wrong best plan properties (#23973)
b2ca281395 is described below

commit b2ca28139553cacd0f5f2e492e30d3a37ce26bed
Author: morrySnow <101034200+morrys...@users.noreply.github.com>
AuthorDate: Thu Sep 7 20:12:53 2023 +0800

    [fix](Nereids) record wrong best plan properties (#23973)
    
    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=[[ [...]
         [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
    ```
---
 .../java/org/apache/doris/nereids/NereidsPlanner.java | 19 +++++--------------
 .../java/org/apache/doris/nereids/memo/Group.java     | 17 ++++++++++++++++-
 .../main/java/org/apache/doris/nereids/memo/Memo.java | 18 +-----------------
 .../properties/EnforceMissingPropertiesHelper.java    |  2 +-
 4 files changed, 23 insertions(+), 33 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
index 6dc75f2632..9d3c836496 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
@@ -72,9 +72,6 @@ import java.util.Collections;
 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;
 
 /**
@@ -335,20 +332,14 @@ public class NereidsPlanner extends Planner {
                     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
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java
index 4b227da3c5..750de4fa8f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Group.java
@@ -438,7 +438,7 @@ public class Group {
         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");
         }
@@ -446,6 +446,21 @@ public class Group {
             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();
     }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
index 4204f33ca1..d3a5d8a488 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
@@ -779,23 +779,7 @@ public class Memo {
         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();
     }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java
index 1c45db6d4c..d548e3254c 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java
@@ -87,7 +87,7 @@ public class EnforceMissingPropertiesHelper {
         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) {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to