Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/281#discussion_r153574645
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/optimize/QueryOptimizer.java ---
    @@ -91,8 +91,23 @@ public QueryPlan optimize(PhoenixStatement statement, 
SelectStatement select, Co
         }
         
         public QueryPlan optimize(QueryPlan dataPlan, PhoenixStatement 
statement, List<? extends PDatum> targetColumns, ParallelIteratorFactory 
parallelIteratorFactory) throws SQLException {
    -        List<QueryPlan>plans = getApplicablePlans(dataPlan, statement, 
targetColumns, parallelIteratorFactory, true);
    -        return plans.get(0);
    +        List<QueryPlan> plans = getApplicablePlans(dataPlan, statement, 
targetColumns, parallelIteratorFactory, false);
    +        if (plans.size() == 1) {
    +            return plans.get(0);
    +        }
    +
    +        // Get the best plan based on their costs. Costs will be ZERO if 
stats are not
    +        // available, thus the first plan will be returned.
    +        Cost minCost = null;
    +        QueryPlan bestPlan = null;
    +        for (QueryPlan plan : plans) {
    +            Cost cost = plan.getCost();
    +            if (minCost == null || cost.compareTo(minCost) < 0) {
    +                minCost = cost;
    +                bestPlan = plan;
    +            }
    +        }
    +        return bestPlan;
    --- End diff --
    
    I'm still not seeing this change. The only existing method that should 
change in QueryOptimizer is this private one:
    
        private List<QueryPlan> getApplicablePlans(QueryPlan dataPlan, 
PhoenixStatement statement, List<? extends PDatum> targetColumns, 
ParallelIteratorFactory parallelIteratorFactory, boolean stopAtBestPlan) throws 
SQLException {
    
    The only change necessary should be to call a new method that orders the 
plans according to cost.


---

Reply via email to