Return-Path: <dev-return-47033-archive-asf-public=cust-asf.ponee.io@phoenix.apache.org>
X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io
Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io
Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183])
	by cust-asf2.ponee.io (Postfix) with ESMTP id 8238A200D42
	for <archive-asf-public-internal@cust-asf2.ponee.io>; Fri, 17 Nov 2017 09:11:40 +0100 (CET)
Received: by cust-asf.ponee.io (Postfix)
	id 808F3160BFB; Fri, 17 Nov 2017 08:11:40 +0000 (UTC)
Delivered-To: archive-asf-public@cust-asf.ponee.io
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by cust-asf.ponee.io (Postfix) with SMTP id C518D160BF8
	for <archive-asf-public@cust-asf.ponee.io>; Fri, 17 Nov 2017 09:11:39 +0100 (CET)
Received: (qmail 36971 invoked by uid 500); 17 Nov 2017 08:11:38 -0000
Mailing-List: contact dev-help@phoenix.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@phoenix.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@phoenix.apache.org>
List-Post: <mailto:dev@phoenix.apache.org>
List-Id: <dev.phoenix.apache.org>
Reply-To: dev@phoenix.apache.org
Delivered-To: mailing list dev@phoenix.apache.org
Received: (qmail 36901 invoked by uid 99); 17 Nov 2017 08:11:38 -0000
Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 17 Nov 2017 08:11:38 +0000
Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33)
	id AA759DFBC6; Fri, 17 Nov 2017 08:11:37 +0000 (UTC)
From: JamesRTaylor <git@git.apache.org>
To: dev@phoenix.apache.org
Reply-To: dev@phoenix.apache.org
References: <git-pr-281-phoenix@git.apache.org>
In-Reply-To: <git-pr-281-phoenix@git.apache.org>
Subject: [GitHub] phoenix pull request #281: PHOENIX-4288 Indexes not used when ordering by pr...
Content-Type: text/plain
Message-Id: <20171117081137.AA759DFBC6@git1-us-west.apache.org>
Date: Fri, 17 Nov 2017 08:11:37 +0000 (UTC)
archived-at: Fri, 17 Nov 2017 08:11:40 -0000

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

    https://github.com/apache/phoenix/pull/281#discussion_r151623809
  
    --- 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 agree with @katameru. If you plug in your optimization code to impact the sort order done by QueryOptimizer.orderPlansBestToWorst() when getApplicablePlans() is called, we'd keep the behavior that the hints would override the optimizer (whether cost-based is enabled or not).
    
    I still think we need more tests. We've found that if the tests don't get added with the initial commit, they unfortunately never get added. 


---