You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by sa...@apache.org on 2016/12/05 22:25:03 UTC
[5/7] incubator-impala git commit: IMPALA-4303: Do not reset()
qualifier of union operands.
IMPALA-4303: Do not reset() qualifier of union operands.
The bug: We used to reset() the qualifier of union operands
to their original value obtained during parsing. This leads to
problems when union operands are unnested and we need to rewrite
Subqueries. In particular, the first union operand of a nested union
was reset() to a null qualifier, but that operand could be somewhere
in the middle of the list of unnested operands in the parent. At that
point, we've lost information about the qualifier of the unnested
operand.
The fix: The simplest solution is to not reset() the qualifier.
The other alternative is be to reset() the qualifier, but also
undo any unnesting. That seems unnecessary and wasteful.
Change-Id: I157bb0f08c4a94fd779487d7c23edd64a537a1f6
Reviewed-on: http://gerrit.cloudera.org:8080/4963
Reviewed-by: Alex Behm <al...@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/852e272b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/852e272b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/852e272b
Branch: refs/heads/master
Commit: 852e272b32a5424e1ea31af7b3d532dac0f916b3
Parents: 1da5701
Author: Alex Behm <al...@cloudera.com>
Authored: Fri Nov 4 17:52:47 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Mon Dec 5 00:58:30 2016 +0000
----------------------------------------------------------------------
.../org/apache/impala/analysis/UnionStmt.java | 27 +++++----
.../queries/PlannerTest/subquery-rewrite.test | 59 ++++++++++++++++++++
2 files changed, 74 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/852e272b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
index d5f1ef1..a0e7164 100644
--- a/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
@@ -52,23 +52,19 @@ public class UnionStmt extends QueryStmt {
}
/**
- * Represents an operand to a union, created by the parser.
- * Contains a query statement and the all/distinct qualifier
- * of the union operator (null for the first queryStmt).
+ * Represents an operand to a union. It consists of a query statement and its left
+ * all/distinct qualifier (null for the first operand).
*/
public static class UnionOperand {
- // Qualifier as seen by the parser. Null for the first operand.
- private final Qualifier originalQualifier_;
+ // Effective qualifier. Should not be reset() to preserve changes made during
+ // distinct propagation and unnesting that are needed after rewriting Subqueries.
+ private Qualifier qualifier_;
/////////////////////////////////////////
// BEGIN: Members that need to be reset()
private final QueryStmt queryStmt_;
- // Effective qualifier. Possibly different from parsedQualifier_ due
- // to DISTINCT propagation.
- private Qualifier qualifier_;
-
// Analyzer used for this operand. Set in analyze().
// We must preserve the conjuncts registered in the analyzer for partition pruning.
private Analyzer analyzer_;
@@ -81,7 +77,6 @@ public class UnionStmt extends QueryStmt {
public UnionOperand(QueryStmt queryStmt, Qualifier qualifier) {
queryStmt_ = queryStmt;
- originalQualifier_ = qualifier;
qualifier_ = qualifier;
smap_ = new ExprSubstitutionMap();
}
@@ -114,7 +109,6 @@ public class UnionStmt extends QueryStmt {
*/
private UnionOperand(UnionOperand other) {
queryStmt_ = other.queryStmt_.clone();
- originalQualifier_ = other.originalQualifier_;
qualifier_ = other.qualifier_;
analyzer_ = other.analyzer_;
smap_ = other.smap_.clone();
@@ -122,7 +116,6 @@ public class UnionStmt extends QueryStmt {
public void reset() {
queryStmt_.reset();
- qualifier_ = originalQualifier_;
analyzer_ = null;
smap_.clear();
}
@@ -373,6 +366,9 @@ public class UnionStmt extends QueryStmt {
unnestOperand(allOperands_, Qualifier.ALL, operands_.get(i));
}
+ for (UnionOperand op: distinctOperands_) op.setQualifier(Qualifier.DISTINCT);
+ for (UnionOperand op: allOperands_) op.setQualifier(Qualifier.ALL);
+
operands_.clear();
operands_.addAll(distinctOperands_);
operands_.addAll(allOperands_);
@@ -608,6 +604,13 @@ public class UnionStmt extends QueryStmt {
@Override
public UnionStmt clone() { return new UnionStmt(this); }
+ /**
+ * Undoes all changes made by analyze() except distinct propagation and unnesting.
+ * After analysis, operands_ contains the list of unnested operands with qualifiers
+ * adjusted to reflect distinct propagation. Every operand in that list is reset().
+ * The distinctOperands_ and allOperands_ are cleared because they are redundant
+ * with operands_.
+ */
@Override
public void reset() {
super.reset();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/852e272b/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test b/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
index 85559b6..cf723d2 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
@@ -2053,3 +2053,62 @@ PLAN-ROOT SINK
|
00:EMPTYSET
====
+# IMPALA-4303: Test subquery rewriting with nested unions.
+select * from functional.alltypestiny
+where exists (select 1 from functional.alltypes where int_col < 10)
+union all
+ (select * from functional.alltypestiny where year=2009 and month=1
+ union all
+ select * from functional.alltypestiny where year=2009 and month=2)
+---- PLAN
+PLAN-ROOT SINK
+|
+00:UNION
+|
+|--05:SCAN HDFS [functional.alltypestiny]
+| partitions=1/4 files=1 size=115B
+|
+|--04:SCAN HDFS [functional.alltypestiny]
+| partitions=1/4 files=1 size=115B
+|
+03:NESTED LOOP JOIN [LEFT SEMI JOIN]
+|
+|--02:SCAN HDFS [functional.alltypes]
+| partitions=24/24 files=24 size=478.45KB
+| predicates: int_col < 10
+| limit: 1
+|
+01:SCAN HDFS [functional.alltypestiny]
+ partitions=4/4 files=4 size=460B
+====
+# IMPALA-4303: Test subquery rewriting with nested unions.
+select * from functional.alltypestiny
+where exists (select 1 from functional.alltypes where int_col < 10)
+union distinct
+ (select * from functional.alltypestiny where year=2009 and month=1
+ union distinct
+ select * from functional.alltypestiny where year=2009 and month=2)
+---- PLAN
+PLAN-ROOT SINK
+|
+06:AGGREGATE [FINALIZE]
+| group by: id, bool_col, tinyint_col, smallint_col, int_col, bigint_col, float_col, double_col, date_string_col, string_col, timestamp_col, year, month
+|
+00:UNION
+|
+|--05:SCAN HDFS [functional.alltypestiny]
+| partitions=1/4 files=1 size=115B
+|
+|--04:SCAN HDFS [functional.alltypestiny]
+| partitions=1/4 files=1 size=115B
+|
+03:NESTED LOOP JOIN [LEFT SEMI JOIN]
+|
+|--02:SCAN HDFS [functional.alltypes]
+| partitions=24/24 files=24 size=478.45KB
+| predicates: int_col < 10
+| limit: 1
+|
+01:SCAN HDFS [functional.alltypestiny]
+ partitions=4/4 files=4 size=460B
+====