IMPALA-4470: Avoid creating a NumericLiteral from NaN/infinity/-0. Our NumericLiteral is backed by a BigDecimal which cannot represent the special float values NaN, infinity or negative zero. As a result, when evaluating constant expressions from the FE we hit an exception when trying to create a NumericLiteral from a NaN or infinity value. Before, negative zero would silently get converted to zero which is dangerous.
The fix is to treat the expr evaluation as a failure and not replace the constant Expr with a LiteralExpr. Change-Id: I8243b2ee9fa9c470d078b385583f2f48b606a230 Reviewed-on: http://gerrit.cloudera.org:8080/5050 Reviewed-by: Alex Behm <[email protected]> Tested-by: Internal 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/f5e660dd Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/f5e660dd Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/f5e660dd Branch: refs/heads/master Commit: f5e660dd6eabc667094d8bc40810602cd7fa56d4 Parents: 107fc4e Author: Alex Behm <[email protected]> Authored: Thu Nov 10 13:50:20 2016 -0800 Committer: Internal Jenkins <[email protected]> Committed: Wed Nov 16 23:55:42 2016 +0000 ---------------------------------------------------------------------- .../java/org/apache/impala/analysis/LiteralExpr.java | 9 ++++++--- .../org/apache/impala/analysis/NumericLiteral.java | 15 +++++++++++++++ .../functional-planner/queries/PlannerTest/hdfs.test | 11 +++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f5e660dd/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 de73770..4ace2dc 100644 --- a/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java @@ -151,7 +151,9 @@ public abstract class LiteralExpr extends Expr implements Comparable<LiteralExpr /** * Evaluates the given constant expr and returns its result as a LiteralExpr. * Assumes expr has been analyzed. Returns constExpr if is it already a LiteralExpr. - * Returns null for types that do not have a LiteralExpr subclass, e.g. TIMESTAMP. + * Returns null for types that do not have a LiteralExpr subclass, e.g. TIMESTAMP, or + * in cases where the corresponding LiteralExpr is not able to represent the evaluation + * result, e.g., NaN or infinity. * TODO: Support non-scalar types. */ public static LiteralExpr create(Expr constExpr, TQueryCtx queryCtx) @@ -199,8 +201,9 @@ public abstract class LiteralExpr extends Expr implements Comparable<LiteralExpr case FLOAT: case DOUBLE: if (val.isSetDouble_val()) { - result = - new NumericLiteral(new BigDecimal(val.double_val), constExpr.getType()); + // A NumericLiteral cannot represent NaN, infinity or negative zero. + if (!NumericLiteral.isValidLiteral(val.double_val)) return null; + result = new NumericLiteral(new BigDecimal(val.double_val), constExpr.getType()); } break; case DECIMAL: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f5e660dd/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 038cd9b..8cf102a 100644 --- a/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java +++ b/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java @@ -29,6 +29,7 @@ import org.apache.impala.thrift.TExprNode; import org.apache.impala.thrift.TExprNodeType; import org.apache.impala.thrift.TFloatLiteral; import org.apache.impala.thrift.TIntLiteral; + import com.google.common.base.Objects; import com.google.common.base.Preconditions; @@ -46,6 +47,8 @@ public class NumericLiteral extends LiteralExpr { // it by 10) does not unnecessarily change the unscaled value. Special care // needs to be taken when converting between the big decimals unscaled value // and ours. (See getUnscaledValue()). + // A BigDecimal cannot represent special float values like NaN, infinity, or + // negative zero. private BigDecimal value_; // If true, this literal has been explicitly cast to a type and should not @@ -109,6 +112,18 @@ public class NumericLiteral extends LiteralExpr { explicitlyCast_ = other.explicitlyCast_; } + /** + * Returns true if 'v' can be represented by a NumericLiteral, false otherwise. + * Special float values like NaN, infinity, and negative zero cannot be represented + * by a NumericLiteral. + */ + public static boolean isValidLiteral(double v) { + if (Double.isNaN(v) || Double.isInfinite(v)) return false; + // Check for negative zero. + if (v == 0 && 1.0 / v == Double.NEGATIVE_INFINITY) return false; + return true; + } + @Override public String debugString() { return Objects.toStringHelper(this) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f5e660dd/testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test b/testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test index 9d43a25..606425b 100644 --- a/testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test +++ b/testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test @@ -1008,3 +1008,14 @@ PLAN-ROOT SINK 00:SCAN HDFS [functional.alltypesagg t1] partitions=3/11 files=3 size=222.34KB ==== +# IMPALA-4470: Partition pruning with a constant expr that evaluates to NaN/infinity. +# 0 / 0 --> NaN and 1 / 0 --> Infinity +select * from functional.alltypes +where year = (cast(0 as double) / cast(0 as double)) + and month = (cast(1 as float) / cast(0 as float)) +---- PLAN +PLAN-ROOT SINK +| +00:SCAN HDFS [functional.alltypes] + partitions=0/24 files=0 size=0B +====
