You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mj...@apache.org on 2016/08/18 16:30:23 UTC
[1/2] incubator-impala git commit: IMPALA-3992: bad shell error
message when running nonexistent file
Repository: incubator-impala
Updated Branches:
refs/heads/master 532b1fe11 -> 0983da92b
IMPALA-3992: bad shell error message when running nonexistent file
Fix the error handling code and add a test.
Change-Id: Iebcf1dc8a1a08b400a2c769a9cff38ea02c8e525
Reviewed-on: http://gerrit.cloudera.org:8080/4022
Reviewed-by: Henry Robinson <he...@cloudera.com>
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/50e21247
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/50e21247
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/50e21247
Branch: refs/heads/master
Commit: 50e21247d60b9d33cb2601126a1af9231fc7e43b
Parents: 532b1fe
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Aug 17 14:28:45 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Thu Aug 18 03:37:48 2016 +0000
----------------------------------------------------------------------
shell/impala_shell.py | 3 ++-
tests/shell/test_shell_commandline.py | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/50e21247/shell/impala_shell.py
----------------------------------------------------------------------
diff --git a/shell/impala_shell.py b/shell/impala_shell.py
index ff35412..b9dc60a 100755
--- a/shell/impala_shell.py
+++ b/shell/impala_shell.py
@@ -1232,7 +1232,8 @@ def execute_queries_non_interactive_mode(options):
else:
query_file_handle = open(options.query_file, 'r')
except Exception, e:
- print_to_stderr("Could not open file '%s': %s", options.query_file, e)
+ print_to_stderr("Could not open file '%s': %s" % (options.query_file, e))
+ sys.exit(1)
query_text = query_file_handle.read()
elif options.query:
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/50e21247/tests/shell/test_shell_commandline.py
----------------------------------------------------------------------
diff --git a/tests/shell/test_shell_commandline.py b/tests/shell/test_shell_commandline.py
index 9963730..29c5040 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -429,3 +429,6 @@ class TestImpalaShell(ImpalaTestSuite):
assert "(Coordinator: " in results.stderr
assert "Query progress can be monitored at: " in results.stderr
+ def test_missing_query_file(self):
+ result = run_impala_shell_cmd('-f nonexistent.sql', expect_success=False)
+ assert "Could not open file 'nonexistent.sql'" in result.stderr
[2/2] incubator-impala git commit: IMPALA-3856,
IMPALA-3871: Fix BinaryPredicate normalization for Kudu
Posted by mj...@apache.org.
IMPALA-3856,IMPALA-3871: Fix BinaryPredicate normalization for Kudu
Change-Id: Iae7612433a2e27f8887abe6624f9ee0f4867b934
Reviewed-on: http://gerrit.cloudera.org:8080/3986
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
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/0983da92
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/0983da92
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/0983da92
Branch: refs/heads/master
Commit: 0983da92bad9407c864f884973f9530e0059036d
Parents: 50e2124
Author: Matthew Jacobs <mj...@cloudera.com>
Authored: Mon Jul 18 12:42:26 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Thu Aug 18 04:03:00 2016 +0000
----------------------------------------------------------------------
.../impala/analysis/BinaryPredicate.java | 31 +++++++++++++-------
.../java/com/cloudera/impala/analysis/Expr.java | 15 +++++-----
.../cloudera/impala/analysis/LiteralExpr.java | 10 +++----
.../queries/PlannerTest/kudu.test | 21 +++++++++++++
4 files changed, 54 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0983da92/fe/src/main/java/com/cloudera/impala/analysis/BinaryPredicate.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/com/cloudera/impala/analysis/BinaryPredicate.java b/fe/src/main/java/com/cloudera/impala/analysis/BinaryPredicate.java
index 02b5c12..35d03e1 100644
--- a/fe/src/main/java/com/cloudera/impala/analysis/BinaryPredicate.java
+++ b/fe/src/main/java/com/cloudera/impala/analysis/BinaryPredicate.java
@@ -115,24 +115,35 @@ public class BinaryPredicate extends Predicate {
}
/**
- * Returns a version of 'predicate' that always has the SlotRef, if there is one, on the
- * left and the other expression on the right. This also folds constant children of the
- * predicate into literals, if possible.
- * Returns null if this is not a SlotRef comparison.
+ * Normalizes a 'predicate' consisting of an uncast SlotRef and a constant Expr into
+ * the following form: <SlotRef> <Op> <LiteralExpr>
+ * If 'predicate' cannot be expressed in this way, null is returned.
*/
public static BinaryPredicate normalizeSlotRefComparison(BinaryPredicate predicate,
- Analyzer analyzer)
- throws AnalysisException {
- SlotRef ref = predicate.getBoundSlot();
+ Analyzer analyzer) {
+ SlotRef ref = null;
+ if (predicate.getChild(0) instanceof SlotRef) {
+ ref = (SlotRef) predicate.getChild(0);
+ } else if (predicate.getChild(1) instanceof SlotRef) {
+ ref = (SlotRef) predicate.getChild(1);
+ }
+
if (ref == null) return null;
- if (ref != predicate.getChild(0).unwrapSlotRef(true)) {
- Preconditions.checkState(ref == predicate.getChild(1).unwrapSlotRef(true));
+ if (ref != predicate.getChild(0)) {
+ Preconditions.checkState(ref == predicate.getChild(1));
predicate = new BinaryPredicate(predicate.getOp().converse(), ref,
predicate.getChild(0));
predicate.analyzeNoThrow(analyzer);
}
- predicate.foldConstantChildren(analyzer);
+
+ try {
+ predicate.foldConstantChildren(analyzer);
+ } catch (AnalysisException ex) {
+ // Throws if the expression cannot be evaluated by the BE.
+ return null;
+ }
predicate.analyzeNoThrow(analyzer);
+ if (!(predicate.getChild(1) instanceof LiteralExpr)) return null;
return predicate;
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0983da92/fe/src/main/java/com/cloudera/impala/analysis/Expr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/com/cloudera/impala/analysis/Expr.java b/fe/src/main/java/com/cloudera/impala/analysis/Expr.java
index 12763f7..91d240a 100644
--- a/fe/src/main/java/com/cloudera/impala/analysis/Expr.java
+++ b/fe/src/main/java/com/cloudera/impala/analysis/Expr.java
@@ -1190,16 +1190,15 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
}
/**
- * Evaluate in the BE the children of 'this' that are constant expressions and
- * substitute them with the evaluation result as LiteralExprs. Modifies 'this'
- * in place and does not re-analyze it. Hence, it is not safe to evaluate the
- * modified expr in the BE as the resolved fn_ may be incorrect given the new
- * arguments.
+ * For children of 'this' that are constant expressions capable of being expressed
+ * as LiteralExprs, evaluate them in the BE and substitute the child with the
+ * resulting LiteralExpr. Modifies 'this' in place and does not re-analyze it. Hence,
+ * it is not safe to evaluate the modified expr in the BE as the resolved fn_ may be
+ * incorrect given the new arguments.
*
* Throws an AnalysisException if the evaluation fails in the BE.
*
- * TODO: Used only during partition pruning. Convert it to a generic constant
- * expression folding function to be used during the analysis.
+ * TODO: Convert to a generic constant expr folding function to be used during analysis.
*/
public void foldConstantChildren(Analyzer analyzer) throws AnalysisException {
Preconditions.checkState(isAnalyzed_);
@@ -1208,7 +1207,7 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
Expr child = getChild(i);
if (child.isLiteral() || !child.isConstant()) continue;
LiteralExpr literalExpr = LiteralExpr.create(child, analyzer.getQueryCtx());
- Preconditions.checkNotNull(literalExpr);
+ if (literalExpr == null) continue;
setChild(i, literalExpr);
}
isAnalyzed_ = false;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0983da92/fe/src/main/java/com/cloudera/impala/analysis/LiteralExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/com/cloudera/impala/analysis/LiteralExpr.java b/fe/src/main/java/com/cloudera/impala/analysis/LiteralExpr.java
index a48b46c..d387b54 100644
--- a/fe/src/main/java/com/cloudera/impala/analysis/LiteralExpr.java
+++ b/fe/src/main/java/com/cloudera/impala/analysis/LiteralExpr.java
@@ -49,7 +49,8 @@ public abstract class LiteralExpr extends Expr implements Comparable<LiteralExpr
}
/**
- * Returns an analyzed literal of 'type'.
+ * Returns an analyzed literal of 'type'. Returns null for types that cannot be
+ * expressed as literals, e.g. TIMESTAMP.
*/
public static LiteralExpr create(String value, Type type) throws AnalysisException {
Preconditions.checkArgument(type.isValid());
@@ -79,8 +80,7 @@ public abstract class LiteralExpr extends Expr implements Comparable<LiteralExpr
case DATETIME:
case TIMESTAMP:
// TODO: we support TIMESTAMP but no way to specify it in SQL.
- throw new AnalysisException(
- "DATE/DATETIME/TIMESTAMP literals not supported: " + value);
+ return null;
default:
Preconditions.checkState(false,
String.format("Literals of type '%s' not supported.", type.toSql()));
@@ -151,6 +151,7 @@ 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 cannot be expressed as literals, e.g. TIMESTAMP.
* TODO: Support non-scalar types.
*/
public static LiteralExpr create(Expr constExpr, TQueryCtx queryCtx)
@@ -216,8 +217,7 @@ public abstract class LiteralExpr extends Expr implements Comparable<LiteralExpr
case DATE:
case DATETIME:
case TIMESTAMP:
- throw new AnalysisException(
- "DATE/DATETIME/TIMESTAMP literals not supported: " + constExpr.toSql());
+ return null;
default:
Preconditions.checkState(false,
String.format("Literals of type '%s' not supported.",
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/0983da92/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test b/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
index 6118c00..7d139d2 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
@@ -147,3 +147,24 @@ NODE 0:
00:SCAN KUDU [functional_kudu.testtbl]
predicates: name IS NULL, CAST(sin(id) AS BOOLEAN) = TRUE
====
+# IMPALA-3856: KuduScanNode crash when pushing predicates including a cast
+select o_orderkey from tpch_kudu.orders where o_orderkey < 10.0 order by 1
+---- PLAN
+01:SORT
+| order by: o_orderkey ASC
+|
+00:SCAN KUDU [tpch_kudu.orders]
+ predicates: o_orderkey < 10.0
+====
+# IMPALA-3871: Casting literals to TIMESTAMP throw when pushed to KuduScanNode
+select t.c from
+ (select cast(o_orderdate as timestamp) c from tpch_kudu.orders where o_orderkey < 10) t
+where t.c <= cast('1995-01-01 00:00:00' as timestamp) order by c
+---- PLAN
+01:SORT
+| order by: c ASC
+|
+00:SCAN KUDU [tpch_kudu.orders]
+ predicates: CAST(o_orderdate AS TIMESTAMP) <= CAST('1995-01-01 00:00:00' AS TIMESTAMP)
+ kudu predicates: o_orderkey <= 9
+====