starocean999 commented on code in PR #16436:
URL: https://github.com/apache/doris/pull/16436#discussion_r1104041354


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/joinorder/hypergraph/GraphSimplifier.java:
##########
@@ -416,35 +422,32 @@ private boolean tryGetSuperset(long bitmap1, long 
bitmap2, List<Long> superset)
         return false;
     }
 
-    private double getSimpleCost(Plan plan) {
-        if (!(plan instanceof LogicalJoin)) {
-            return 
plan.getGroupExpression().get().getOwnerGroup().getStatistics().getRowCount();
-        }
-        return 
plan.getGroupExpression().get().getCostByProperties(PhysicalProperties.ANY);
-    }
-
-    private LogicalJoin simulateJoin(Plan plan1, LogicalJoin join, Plan plan2) 
{
-        // In Graph Simplifier, we use the simple cost model, that is
-        //      Plan.cost = Plan.rowCount + Plan.children1.cost + 
Plan.children2.cost
-        // The reason is that this cost model has ASI (adjacent sequence 
interchange) property.
-        // TODO: consider network, data distribution cost
-        LogicalJoin newJoin = new LogicalJoin<>(join.getJoinType(), plan1, 
plan2);
-        List<Group> children = new ArrayList<>();
-        children.add(getGroup(plan1));
-        children.add(getGroup(plan2));
-        double cost = getSimpleCost(plan1) + getSimpleCost(plan2);
-        GroupExpression groupExpression = new GroupExpression(newJoin, 
children);
-        // FIXME: use wrong constructor
-        Group group = new Group(null, groupExpression, null);
-        StatsCalculator.estimate(groupExpression);
-        cost += group.getStatistics().getRowCount();
-
-        List<PhysicalProperties> inputs = new ArrayList<>();
-        inputs.add(PhysicalProperties.ANY);
-        inputs.add(PhysicalProperties.ANY);
-        groupExpression.updateLowestCostTable(PhysicalProperties.ANY, inputs, 
cost);
-
-        return newJoin.withGroupExpression(Optional.of(groupExpression));
+    private double calCost(Edge edge, StatsDeriveResult stats,
+            StatsDeriveResult leftStats, StatsDeriveResult rightStats) {
+        LogicalJoin join = edge.getJoin();
+        PlanContext planContext = new PlanContext(stats, leftStats, 
rightStats);
+        double cost = 0;
+        if (JoinUtils.shouldNestedLoopJoin(join)) {

Review Comment:
   looks like should swap the "if else" logic?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to