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

    https://github.com/apache/phoenix/pull/281#discussion_r153378386
  
    --- 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 --
    
    QueryOptimize.optimize() is not the only entry point to the optimizer. For 
example, DeleteCompiler calls both getApplicablePlans() and getBestPlan(). I 
think it'd be best if you hid this logic at the end of the private 
getApplicablePlans since all calls channel through that. That way all optimizer 
decisions will use cost if it's enabled. You could conditional order the plans 
based on their cost if cost-based is enabled here:
    
                return hintedPlan != null ? plans : isCostBasedEnabled ? 
orderPlansBasedOnCost(plans) : orderPlansBestToWorst(select, plans, 
stopAtBestPlan);



---

Reply via email to