You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jb...@apache.org on 2017/09/07 03:50:24 UTC

[6/8] incubator-impala git commit: IMPALA-4620: Refactor evalcost computation in query analysis

IMPALA-4620: Refactor evalcost computation in query analysis

This patch adds a computeEvalCost abstract function to class Expr and
call it explicitly when an expr is analyzed. Existing code sets evalcost
at random places in analyzeImpl(), causing a bug casting child expr
without recomputing evalcost. Furthermore, if a child of an Expr is
substituted from one with known evalcost to one with unkwown evalcost,
evalcost of the parent won't be updatde by subsequent analyze() calls.
This patch fixes these problems.
A planner test case with wrong predicate order is also fixed.

Change-Id: Ibec3d648532c185d4318476796b1ba432b0fe59e
Reviewed-on: http://gerrit.cloudera.org:8080/7948
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/897f025c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/897f025c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/897f025c

Branch: refs/heads/master
Commit: 897f025c69bd0dd45520721a0d6eb9c9f14aa342
Parents: f538b43
Author: Tianyi Wang <tw...@cloudera.com>
Authored: Tue Aug 29 19:06:12 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Sep 7 01:47:41 2017 +0000

----------------------------------------------------------------------
 .../apache/impala/analysis/AnalyticExpr.java    |  5 ++--
 .../apache/impala/analysis/ArithmeticExpr.java  |  6 +++-
 .../impala/analysis/BetweenPredicate.java       |  3 ++
 .../apache/impala/analysis/BinaryPredicate.java | 25 ++++++++--------
 .../org/apache/impala/analysis/BoolLiteral.java |  2 --
 .../org/apache/impala/analysis/CaseExpr.java    | 14 ++++-----
 .../org/apache/impala/analysis/CastExpr.java    |  8 ++++--
 .../impala/analysis/CompoundPredicate.java      |  6 +++-
 .../apache/impala/analysis/ExistsPredicate.java |  3 ++
 .../java/org/apache/impala/analysis/Expr.java   |  9 +++++-
 .../impala/analysis/FunctionCallExpr.java       |  5 +++-
 .../org/apache/impala/analysis/InPredicate.java | 10 ++++---
 .../impala/analysis/IsNotEmptyPredicate.java    |  7 ++++-
 .../apache/impala/analysis/IsNullPredicate.java |  6 +++-
 .../impala/analysis/KuduPartitionExpr.java      |  3 ++
 .../apache/impala/analysis/LikePredicate.java   | 30 +++++++++++---------
 .../org/apache/impala/analysis/LiteralExpr.java |  6 ++++
 .../org/apache/impala/analysis/NullLiteral.java |  1 -
 .../apache/impala/analysis/NumericLiteral.java  |  3 --
 .../org/apache/impala/analysis/SlotRef.java     |  6 +++-
 .../apache/impala/analysis/StringLiteral.java   |  1 -
 .../org/apache/impala/analysis/Subquery.java    |  3 ++
 .../analysis/TimestampArithmeticExpr.java       |  6 +++-
 .../impala/analysis/TimestampLiteral.java       |  1 -
 .../impala/analysis/TupleIsNullPredicate.java   |  6 +++-
 .../queries/PlannerTest/inline-view.test        |  4 +--
 26 files changed, 117 insertions(+), 62 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
index 534d09a..15d5b4d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
@@ -35,8 +35,6 @@ import org.apache.impala.service.FeSupport;
 import org.apache.impala.thrift.TColumnValue;
 import org.apache.impala.thrift.TExprNode;
 import org.apache.impala.util.TColumnValueUtil;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
@@ -554,6 +552,9 @@ public class AnalyticExpr extends Expr {
     setChildren();
   }
 
+  @Override
+  protected float computeEvalCost() { return UNKNOWN_COST; }
+
   /**
    * If necessary, rewrites the analytic function, window, and/or order-by elements into
    * a standard format for the purpose of simpler backend execution, as follows:

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java b/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
index 763a38d..48ac2f2 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
@@ -190,7 +190,6 @@ public class ArithmeticExpr extends Expr {
       Preconditions.checkState(children_.size() == 2);
       t1 = getChild(1).getType();
     }
-    if (hasChildCosts()) evalCost_ = getChildCosts() + ARITHMETIC_OP_COST;
 
     String fnName = op_.getName();
     switch (op_) {
@@ -263,5 +262,10 @@ public class ArithmeticExpr extends Expr {
   }
 
   @Override
+  protected float computeEvalCost() {
+    return hasChildCosts() ? getChildCosts() + ARITHMETIC_OP_COST : UNKNOWN_COST;
+  }
+
+  @Override
   public Expr clone() { return new ArithmeticExpr(this); }
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java b/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
index 8806990..4cedeac 100644
--- a/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
@@ -57,6 +57,9 @@ public class BetweenPredicate extends Predicate {
     analyzer.castAllToCompatibleType(children_);
   }
 
+  @Override
+  protected float computeEvalCost() { return UNKNOWN_COST; }
+
   public boolean isNotBetween() { return isNotBetween_; }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java b/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
index cc4b030..afdfaa6 100644
--- a/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
@@ -228,19 +228,20 @@ public class BinaryPredicate extends Predicate {
         selectivity_ = Math.max(0, Math.min(1, selectivity_));
       }
     }
+  }
 
-    // Compute cost.
-    if (hasChildCosts()) {
-      if (getChild(0).getType().isFixedLengthType()) {
-        evalCost_ = getChildCosts() + BINARY_PREDICATE_COST;
-      } else if (getChild(0).getType().isStringType()) {
-        evalCost_ = getChildCosts() +
-            (float) (getAvgStringLength(getChild(0)) + getAvgStringLength(getChild(1)) *
-            BINARY_PREDICATE_COST);
-      } else {
-        //TODO(tmarshall): Handle other var length types here.
-        evalCost_ = getChildCosts() + VAR_LEN_BINARY_PREDICATE_COST;
-      }
+  @Override
+  protected float computeEvalCost() {
+    if (!hasChildCosts()) return UNKNOWN_COST;
+    if (getChild(0).getType().isFixedLengthType()) {
+      return getChildCosts() + BINARY_PREDICATE_COST;
+    } else if (getChild(0).getType().isStringType()) {
+      return getChildCosts() +
+          (float) (getAvgStringLength(getChild(0)) + getAvgStringLength(getChild(1))) *
+              BINARY_PREDICATE_COST;
+    } else {
+      //TODO(tmarshall): Handle other var length types here.
+      return getChildCosts() + VAR_LEN_BINARY_PREDICATE_COST;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java b/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
index 0198704..838f120 100644
--- a/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
@@ -31,12 +31,10 @@ public class BoolLiteral extends LiteralExpr {
   public BoolLiteral(boolean value) {
     this.value_ = value;
     type_ = Type.BOOLEAN;
-    evalCost_ = LITERAL_COST;
   }
 
   public BoolLiteral(String value) throws AnalysisException {
     type_ = Type.BOOLEAN;
-    evalCost_ = LITERAL_COST;
     if (value.toLowerCase().equals("true")) {
       this.value_ = true;
     } else if (value.toLowerCase().equals("false")) {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java b/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
index 9d6ea69..237076a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
@@ -345,18 +345,16 @@ public class CaseExpr extends Expr {
         CompareMode.IS_NONSTRICT_SUPERTYPE_OF);
     Preconditions.checkNotNull(fn_);
     type_ = returnType;
+  }
 
+  @Override
+  protected float computeEvalCost() {
+    if (!hasChildCosts()) return UNKNOWN_COST;
     // Compute cost as the sum of evaluating all of the WHEN exprs, plus
     // the max of the THEN/ELSE exprs.
     float maxThenCost = 0;
     float whenCosts = 0;
-    boolean hasChildCosts = true;
     for (int i = 0; i < children_.size(); ++i) {
-      if (!getChild(i).hasCost()) {
-        hasChildCosts = false;
-        break;
-      }
-
       if (hasCaseExpr_ && i % 2 == 1) {
         // This child is a WHEN expr. BINARY_PREDICATE_COST accounts for the cost of
         // comparing the CASE expr to the WHEN expr.
@@ -371,9 +369,7 @@ public class CaseExpr extends Expr {
         if (thenCost > maxThenCost) maxThenCost = thenCost;
       }
     }
-    if (hasChildCosts) {
-      evalCost_ =  whenCosts + maxThenCost;
-    }
+    return whenCosts + maxThenCost;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
index 3619c46..f185696 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
@@ -66,6 +66,7 @@ public class CastExpr extends Expr {
     try {
       analyze();
       computeNumDistinctValues();
+      evalCost_ = computeEvalCost();
     } catch (AnalysisException ex) {
       Preconditions.checkState(false,
           "Implicit casts should never throw analysis exception.");
@@ -205,9 +206,12 @@ public class CastExpr extends Expr {
     analyze();
   }
 
-  private void analyze() throws AnalysisException {
-    if (getChild(0).hasCost()) evalCost_ = getChild(0).getCost() + CAST_COST;
+  @Override
+  protected float computeEvalCost() {
+    return getChild(0).hasCost() ? getChild(0).getCost() + CAST_COST : UNKNOWN_COST;
+  }
 
+  private void analyze() throws AnalysisException {
     Preconditions.checkNotNull(type_);
     if (type_.isComplexType()) {
       throw new AnalysisException(

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java b/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
index 6354004..ba1ef8d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
@@ -134,7 +134,6 @@ public class CompoundPredicate extends Predicate {
     Preconditions.checkState(fn_ != null);
     Preconditions.checkState(fn_.getReturnType().isBoolean());
     castForFunctionCall(false);
-    if (hasChildCosts()) evalCost_ = getChildCosts() + COMPOUND_PREDICATE_COST;
 
     if (!getChild(0).hasSelectivity() ||
         (children_.size() == 2 && !getChild(1).hasSelectivity())) {
@@ -158,6 +157,11 @@ public class CompoundPredicate extends Predicate {
     selectivity_ = Math.max(0.0, Math.min(1.0, selectivity_));
   }
 
+  @Override
+  protected float computeEvalCost() {
+    return hasChildCosts() ? getChildCosts() + COMPOUND_PREDICATE_COST : UNKNOWN_COST;
+  }
+
   /**
    * Negates a CompoundPredicate.
    */

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java b/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
index 355f562..5acd04a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
@@ -70,6 +70,9 @@ public class ExistsPredicate extends Predicate {
   public Expr clone() { return new ExistsPredicate(this); }
 
   @Override
+  protected float computeEvalCost() { return UNKNOWN_COST; }
+
+  @Override
   public String toSqlImpl() {
     StringBuilder strBuilder = new StringBuilder();
     if (notExists_) strBuilder.append("NOT ");

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/Expr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java
index 774d7b3..5dc0438 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -26,7 +26,6 @@ import java.util.ListIterator;
 import java.util.Set;
 
 import org.apache.impala.analysis.BinaryPredicate.Operator;
-import org.apache.impala.analysis.BoolLiteral;
 import org.apache.impala.catalog.Catalog;
 import org.apache.impala.catalog.Function;
 import org.apache.impala.catalog.Function.CompareMode;
@@ -86,6 +85,7 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
   public final static float LITERAL_COST = 1;
   public final static float SLOT_REF_COST = 1;
   public final static float TIMESTAMP_ARITHMETIC_COST = 5;
+  public final static float UNKNOWN_COST = -1;
 
   // To be used when estimating the cost of Exprs of type string where we don't otherwise
   // have an estimate of how long the strings produced by that Expr are.
@@ -341,6 +341,7 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
 
     // Do all the analysis for the expr subclass before marking the Expr analyzed.
     analyzeImpl(analyzer);
+    evalCost_ = computeEvalCost();
     analysisDone();
   }
 
@@ -363,6 +364,12 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
     }
   }
 
+  /**
+   * Compute and return evalcost of this expr given the evalcost of all children has been
+   * computed. Should be called bottom-up whenever the structure of subtree is modified.
+   */
+  abstract protected float computeEvalCost();
+
   protected void computeNumDistinctValues() {
     if (isConstant()) {
       numDistinctValues_ = 1;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
index 9364f12..5de6f6b 100644
--- a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
@@ -569,9 +569,12 @@ public class FunctionCallExpr extends Expr {
     if (type_.isWildcardChar() || type_.isWildcardVarchar()) {
       type_ = ScalarType.STRING;
     }
+  }
 
+  @Override
+  protected float computeEvalCost() {
     // TODO(tmarshall): Differentiate based on the specific function.
-    if (hasChildCosts()) evalCost_ = getChildCosts() + FUNCTION_CALL_COST;
+    return hasChildCosts() ? getChildCosts() + FUNCTION_CALL_COST : UNKNOWN_COST;
   }
 
   public FunctionCallExpr getMergeAggInputFn() { return mergeAggInputFn_; }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/InPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/InPredicate.java b/fe/src/main/java/org/apache/impala/analysis/InPredicate.java
index de12167..e34752f 100644
--- a/fe/src/main/java/org/apache/impala/analysis/InPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/InPredicate.java
@@ -184,11 +184,13 @@ public class InPredicate extends Predicate {
       }
       selectivity_ = Math.max(0.0, Math.min(1.0, selectivity_));
     }
+  }
 
-    if (hasChildCosts()) {
-      // BINARY_PREDICATE_COST accounts for the cost of performing the comparison.
-      evalCost_ = getChildCosts() + BINARY_PREDICATE_COST * (children_.size() - 1);
-    }
+  @Override
+  protected float computeEvalCost() {
+    if (!hasChildCosts()) return UNKNOWN_COST;
+    // BINARY_PREDICATE_COST accounts for the cost of performing the comparison.
+    return getChildCosts() + BINARY_PREDICATE_COST * (children_.size() - 1);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java b/fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
index 380fcf4..9ea1122 100644
--- a/fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
@@ -48,7 +48,12 @@ public class IsNotEmptyPredicate extends Predicate {
     }
     // Avoid influencing cardinality estimates.
     selectivity_ = 1.0;
-    if (getChild(0).hasCost()) evalCost_ = getChild(0).getCost() + IS_NOT_EMPTY_COST;
+  }
+
+  @Override
+  protected float computeEvalCost() {
+    if (!getChild(0).hasCost()) return UNKNOWN_COST;
+    return getChild(0).getCost() + IS_NOT_EMPTY_COST;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java b/fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
index fb3e964..fe160c8 100644
--- a/fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
@@ -134,7 +134,6 @@ public class IsNullPredicate extends Predicate {
       fn_ = getBuiltinFunction(
           analyzer, IS_NULL, collectChildReturnTypes(), CompareMode.IS_IDENTICAL);
     }
-    if (getChild(0).hasCost()) evalCost_ = getChild(0).getCost() + IS_NULL_COST;
 
     // determine selectivity
     // TODO: increase this to make sure we don't end up favoring broadcast joins
@@ -158,6 +157,11 @@ public class IsNullPredicate extends Predicate {
   }
 
   @Override
+  protected float computeEvalCost() {
+    return getChild(0).hasCost() ? getChild(0).getCost() + IS_NULL_COST : UNKNOWN_COST;
+  }
+
+  @Override
   protected void toThrift(TExprNode msg) {
     msg.node_type = TExprNodeType.FUNCTION_CALL;
   }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java b/fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
index 8d52d59..08c53fd 100644
--- a/fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
@@ -79,6 +79,9 @@ public class KuduPartitionExpr extends Expr {
   }
 
   @Override
+  protected float computeEvalCost() { return UNKNOWN_COST; }
+
+  @Override
   protected String toSqlImpl() {
     StringBuilder sb = new StringBuilder("KuduPartition(");
     for (int i = 0; i < children_.size(); ++i) {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java b/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
index 9fab11e..32ccddb 100644
--- a/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
@@ -143,21 +143,23 @@ public class LikePredicate extends Predicate {
       }
     }
     castForFunctionCall(false);
+  }
 
-    if (hasChildCosts()) {
-      if (getChild(1).isLiteral() && !getChild(1).isNullLiteral() &&
-          Pattern.matches("[%_]*[^%_]*[%_]*", ((StringLiteral) getChild(1)).getValue())) {
-        // This pattern only has wildcards as leading or trailing character,
-        // so it is linear.
-        evalCost_ = getChildCosts() +
-            (float) (getAvgStringLength(getChild(0)) + getAvgStringLength(getChild(1)) *
-            BINARY_PREDICATE_COST) + LIKE_COST;
-      } else {
-        // This pattern is more expensive, so calculate its cost as quadratic.
-        evalCost_ = getChildCosts() +
-            (float) (getAvgStringLength(getChild(0)) * getAvgStringLength(getChild(1)) *
-            BINARY_PREDICATE_COST) + LIKE_COST;
-      }
+  @Override
+  protected float computeEvalCost() {
+    if (!hasChildCosts()) return UNKNOWN_COST;
+    if (getChild(1).isLiteral() && !getChild(1).isNullLiteral() &&
+      Pattern.matches("[%_]*[^%_]*[%_]*", ((StringLiteral) getChild(1)).getValue())) {
+      // This pattern only has wildcards as leading or trailing character,
+      // so it is linear.
+      return getChildCosts() +
+          (float) (getAvgStringLength(getChild(0)) + getAvgStringLength(getChild(1)) *
+              BINARY_PREDICATE_COST) + LIKE_COST;
+    } else {
+      // This pattern is more expensive, so calculate its cost as quadratic.
+      return getChildCosts() +
+          (float) (getAvgStringLength(getChild(0)) * getAvgStringLength(getChild(1)) *
+              BINARY_PREDICATE_COST) + LIKE_COST;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java b/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
index 45a65f9..ccc9c7e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
@@ -43,6 +43,7 @@ public abstract class LiteralExpr extends Expr implements Comparable<LiteralExpr
   private final static Logger LOG = LoggerFactory.getLogger(LiteralExpr.class);
 
   public LiteralExpr() {
+    evalCost_ = LITERAL_COST;
     numDistinctValues_ = 1;
   }
 
@@ -101,6 +102,11 @@ public abstract class LiteralExpr extends Expr implements Comparable<LiteralExpr
     // Literals require no analysis.
   }
 
+  @Override
+  protected float computeEvalCost() {
+    return LITERAL_COST;
+  }
+
   /**
    * Returns an analyzed literal from the thrift object.
    */

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java b/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
index 0dcb264..e2ea869 100644
--- a/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
@@ -28,7 +28,6 @@ public class NullLiteral extends LiteralExpr {
 
   public NullLiteral() {
     type_ = Type.NULL;
-    evalCost_ = LITERAL_COST;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java b/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
index 79c906c..98b9dbd 100644
--- a/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
@@ -90,7 +90,6 @@ public class NumericLiteral extends LiteralExpr {
   public NumericLiteral(BigInteger value, Type type) {
     value_ = new BigDecimal(value);
     type_ = type;
-    evalCost_ = LITERAL_COST;
     explicitlyCast_ = true;
     analysisDone();
   }
@@ -98,7 +97,6 @@ public class NumericLiteral extends LiteralExpr {
   public NumericLiteral(BigDecimal value, Type type) {
     value_ = value;
     type_ = type;
-    evalCost_ = LITERAL_COST;
     explicitlyCast_ = true;
     analysisDone();
   }
@@ -222,7 +220,6 @@ public class NumericLiteral extends LiteralExpr {
         }
       }
     }
-    evalCost_ = LITERAL_COST;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
index 21bfd22..f90d7c6 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java
@@ -111,7 +111,6 @@ public class SlotRef extends Expr {
       // HMS string.
       throw new AnalysisException("Unsupported type in '" + toSql() + "'.");
     }
-    evalCost_ = SLOT_REF_COST;
 
     numDistinctValues_ = desc_.getStats().getNumDistinctValues();
     Table rootTable = resolvedPath.getRootTable();
@@ -122,6 +121,11 @@ public class SlotRef extends Expr {
   }
 
   @Override
+  protected float computeEvalCost() {
+    return SLOT_REF_COST;
+  }
+
+  @Override
   protected boolean isConstantImpl() { return false; }
 
   public SlotDescriptor getDesc() {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java b/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
index 4f838ff..1dbe5a4 100644
--- a/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
@@ -47,7 +47,6 @@ public class StringLiteral extends LiteralExpr {
   public StringLiteral(String value, Type type, boolean needsUnescaping) {
     value_ = value;
     type_ = type;
-    evalCost_ = LITERAL_COST;
     needsUnescaping_ = needsUnescaping;
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/Subquery.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Subquery.java b/fe/src/main/java/org/apache/impala/analysis/Subquery.java
index 1626d18..bad3d7e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Subquery.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Subquery.java
@@ -99,6 +99,9 @@ public class Subquery extends Expr {
   }
 
   @Override
+  protected float computeEvalCost() { return UNKNOWN_COST; }
+
+  @Override
   protected boolean isConstantImpl() { return false; }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java b/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
index 104cd1a..5a5468d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
@@ -164,7 +164,11 @@ public class TimestampArithmeticExpr extends Expr {
     Preconditions.checkNotNull(fn_);
     Preconditions.checkState(fn_.getReturnType().isTimestamp());
     type_ = fn_.getReturnType();
-    if (hasChildCosts()) evalCost_ = getChildCosts() + TIMESTAMP_ARITHMETIC_COST;
+  }
+
+  @Override
+  protected float computeEvalCost() {
+    return hasChildCosts() ? getChildCosts() + TIMESTAMP_ARITHMETIC_COST : UNKNOWN_COST;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java b/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
index 568f6e7..25f111e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
@@ -45,7 +45,6 @@ public class TimestampLiteral extends LiteralExpr {
     value_ = value;
     strValue_ = strValue;
     type_ = Type.TIMESTAMP;
-    evalCost_ = Expr.LITERAL_COST;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java b/fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
index f55ed81..8da0237 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
@@ -60,7 +60,11 @@ public class TupleIsNullPredicate extends Predicate {
   protected void analyzeImpl(Analyzer analyzer) throws AnalysisException {
     super.analyzeImpl(analyzer);
     analyzer_ = analyzer;
-    evalCost_ = tupleIds_.size() * IS_NULL_COST;
+  }
+
+  @Override
+  protected float computeEvalCost() {
+    return tupleIds_.size() * IS_NULL_COST;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/897f025c/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test b/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
index 7d10dc6..2c866e3 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
@@ -215,7 +215,7 @@ PLAN-ROOT SINK
 |
 |--01:SCAN HDFS [functional.alltypessmall]
 |     partitions=2/4 files=2 size=3.17KB
-|     predicates: functional.alltypessmall.id + 15 = 27, functional.alltypessmall.string_col = '15'
+|     predicates: functional.alltypessmall.string_col = '15', functional.alltypessmall.id + 15 = 27
 |
 00:SCAN HDFS [functional.alltypesagg]
    partitions=5/11 files=5 size=372.38KB
@@ -235,7 +235,7 @@ PLAN-ROOT SINK
 |  |
 |  01:SCAN HDFS [functional.alltypessmall]
 |     partitions=2/4 files=2 size=3.17KB
-|     predicates: functional.alltypessmall.id + 15 = 27, functional.alltypessmall.string_col = '15'
+|     predicates: functional.alltypessmall.string_col = '15', functional.alltypessmall.id + 15 = 27
 |
 03:EXCHANGE [HASH(id,int_col)]
 |