You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2017/09/13 15:26:47 UTC
[4/6] incubator-impala git commit: IMPALA-5597: Try casting
targetExpr when building runtime filter plan
IMPALA-5597: Try casting targetExpr when building runtime filter plan
This patch fixes a bug that fails a precondition check when generating
runtime filter plans. The lhs and rhs or join predicate might have
different types when the eq predicate function accepts wildcard-typed
parameters. In this case in existing code the types of source and target
expr will be found mismatch and an exception will be thrown when
generating runtime filters. This patch tries to cast target expr to be
of the same type as source expr. A testcase is added to joins.test
Change-Id: I0d66673e280ce13cd3a514236c0c2ecbc50475a6
Reviewed-on: http://gerrit.cloudera.org:8080/7949
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/caa382c2
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/caa382c2
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/caa382c2
Branch: refs/heads/master
Commit: caa382c284fcae6b795c48d53cace6956fc41371
Parents: 34d63e9
Author: Tianyi Wang <tw...@cloudera.com>
Authored: Fri Sep 1 12:49:48 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Sep 13 07:39:03 2017 +0000
----------------------------------------------------------------------
.../apache/impala/analysis/AnalyticExpr.java | 3 +--
.../java/org/apache/impala/analysis/Expr.java | 3 +--
.../impala/analysis/FunctionCallExpr.java | 3 +--
.../impala/planner/RuntimeFilterGenerator.java | 23 +++++++++++---------
.../PlannerTest/runtime-filter-propagation.test | 22 +++++++++++++++++++
.../queries/QueryTest/runtime_row_filters.test | 15 +++++++++++++
6 files changed, 53 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/caa382c2/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 15d5b4d..28de6da 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
@@ -830,8 +830,7 @@ public class AnalyticExpr extends Expr {
}
@Override
- protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer)
- throws AnalysisException {
+ protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer) {
Expr e = super.substituteImpl(smap, analyzer);
if (!(e instanceof AnalyticExpr)) return e;
// Re-sync state after possible child substitution.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/caa382c2/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 5dc0438..2b31b61 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -862,8 +862,7 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
* Exprs that have non-child exprs which should be affected by substitutions must
* override this method and apply the substitution to such exprs as well.
*/
- protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer)
- throws AnalysisException {
+ protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer) {
if (isImplicitCast()) return getChild(0).substituteImpl(smap, analyzer);
if (smap != null) {
Expr substExpr = smap.get(this);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/caa382c2/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 5de6f6b..2bc2860 100644
--- a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
@@ -618,8 +618,7 @@ public class FunctionCallExpr extends Expr {
public Expr clone() { return new FunctionCallExpr(this); }
@Override
- protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer)
- throws AnalysisException {
+ protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer) {
Expr e = super.substituteImpl(smap, analyzer);
if (!(e instanceof FunctionCallExpr)) return e;
FunctionCallExpr fn = (FunctionCallExpr) e;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/caa382c2/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java b/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
index 67c3532..646eb48 100644
--- a/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
+++ b/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
@@ -36,6 +36,8 @@ import org.apache.impala.analysis.TupleDescriptor;
import org.apache.impala.analysis.TupleId;
import org.apache.impala.analysis.TupleIsNullPredicate;
import org.apache.impala.catalog.Table;
+import org.apache.impala.catalog.Type;
+import org.apache.impala.common.AnalysisException;
import org.apache.impala.common.IdGenerator;
import org.apache.impala.planner.PlanNode;
import org.apache.impala.thrift.TRuntimeFilterDesc;
@@ -581,20 +583,21 @@ public final class RuntimeFilterGenerator {
}
Preconditions.checkState(exprSlots.size() == smap.size());
try {
- targetExpr = targetExpr.substitute(smap, analyzer, true);
+ targetExpr = targetExpr.substitute(smap, analyzer, false);
+ } catch (Exception e) {
+ return null;
+ }
+ }
+ Type srcType = filter.getSrcExpr().getType();
+ // Types of targetExpr and srcExpr must be exactly the same since runtime filters are
+ // based on hashing.
+ if (!targetExpr.getType().equals(srcType)) {
+ try {
+ targetExpr = targetExpr.castTo(srcType);
} catch (Exception e) {
- // An exception is thrown if we cannot generate a target expr from this
- // scan node that has the same type as the lhs expr of the join predicate
- // from which the runtime filter was generated. We skip that scan node and will
- // try to assign the filter to a different scan node.
- //
- // TODO: Investigate if we can generate a type-compatible source/target expr
- // pair from that scan node instead of skipping it.
return null;
}
}
- Preconditions.checkState(
- targetExpr.getType().matchesType(filter.getSrcExpr().getType()));
return targetExpr;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/caa382c2/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test b/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
index 602505b..c1675af 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
@@ -1434,3 +1434,25 @@ PLAN-ROOT SINK
partitions=24/24 files=24 size=478.45KB
runtime filters: RF000 -> b.id
====
+# IMPALA-5597: Runtime filter should be generated and assigned successfully when the
+# source expr and target expr have different decimal types.
+select *
+from tpch_parquet.lineitem
+left join tpch_parquet.part on if(l_orderkey % 2 = 0, NULL, l_partkey) = p_partkey
+where l_orderkey = 965 and l_extendedprice * l_tax = p_retailprice;
+---- Plan
+PLAN-ROOT SINK
+|
+02:HASH JOIN [RIGHT OUTER JOIN]
+| hash predicates: p_partkey = if(l_orderkey % 2 = 0, NULL, l_partkey)
+| other predicates: p_retailprice = l_extendedprice * l_tax
+| runtime filters: RF000 <- if(l_orderkey % 2 = 0, NULL, l_partkey), RF001 <- l_extendedprice * l_tax
+|
+|--00:SCAN HDFS [tpch_parquet.lineitem]
+| partitions=1/1 files=3 size=193.71MB
+| predicates: l_orderkey = 965
+|
+01:SCAN HDFS [tpch_parquet.part]
+ partitions=1/1 files=1 size=6.23MB
+ runtime filters: RF000 -> p_partkey, RF001 -> p_retailprice
+====
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/caa382c2/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test b/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
index 01e1055..7cb5884 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
@@ -319,3 +319,18 @@ from tpch_parquet.lineitem l1 join tpch_parquet.lineitem l2
---- RESULTS
0
====
+
+
+---- QUERY
+# Test case 15: filter with a predicate that has different decimal precision between
+# lhs expr and rhs expr.
+# IMPALA-5597: Runtime filter should be generated and assigned successfully when the
+# source expr and target expr have different decimal types.
+
+select count(*)
+from tpch_parquet.lineitem
+left join tpch_parquet.part on if(l_orderkey % 2 = 0, NULL, l_partkey) = p_partkey
+where l_orderkey = 965 and l_extendedprice * l_tax = p_retailprice;
+---- RESULTS
+1
+====