IMPALA-4309: Introduce Expr rewrite phase and supporting classes. Introduces a new phase for rewriting Exprs after analysis and before subquery rewriting. The transformed Exprs replace the original ones in analyzed statements. If Exprs were changed, the whole statement is reset() and re-analyzed, similar to how subqueries are rewritten. If both Exprs and subqueries are rewritten there is only one re-analysis of the changed statement.
The following new classes work together to perform transformations: 1. ExprRewriteRule - base class for Expr transformation rules 2. ExprRewriter - drives the transformation of Exprs using a list of ExprRewriteRules Statements that have exprs to be rewritten need to implement a new method rewriteExprs() that accepts an ExprRewriter. As an example, this patch adds a rule for converting BetweenPredicates into their equivalent CompoundPredicates. The BetweenPredicate has been notoriously buggy due to a lack of such a separate rewrite phase and is now cleaned up. Testing: 1. Added a new test for checking that the rewrite framework covers all relevant statements, clauses and can properly handle nested statements and subqueries. 2. Added a new test for ExprRewriteRules and implemented tests for the BetweenPredicate rewrite. 2. There are many existing tests for BetweePredicates and they all exercise the new rewrite rule/phase. 3. Ran a private core/hdfs run and it passed. Change-Id: I2279dc984bcf7742db4fa3b1aa67283ecbb05e6e Reviewed-on: http://gerrit.cloudera.org:8080/4746 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/1d8cdb02 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/1d8cdb02 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/1d8cdb02 Branch: refs/heads/master Commit: 1d8cdb02c67fad9279a899626f3cd56bce0b0eb3 Parents: 6a16e44 Author: Alex Behm <[email protected]> Authored: Fri Oct 14 11:19:39 2016 -0700 Committer: Internal Jenkins <[email protected]> Committed: Thu Nov 3 20:08:37 2016 +0000 ---------------------------------------------------------------------- .../apache/impala/analysis/AnalysisContext.java | 65 ++++--- .../org/apache/impala/analysis/Analyzer.java | 9 + .../impala/analysis/BetweenPredicate.java | 111 ++---------- .../analysis/CreateOrAlterViewStmtBase.java | 8 +- .../analysis/CreateTableAsSelectStmt.java | 10 +- .../apache/impala/analysis/InlineViewRef.java | 14 +- .../org/apache/impala/analysis/InsertStmt.java | 17 +- .../org/apache/impala/analysis/ModifyStmt.java | 18 +- .../org/apache/impala/analysis/SelectList.java | 11 ++ .../apache/impala/analysis/SelectListItem.java | 3 +- .../org/apache/impala/analysis/SelectStmt.java | 32 +++- .../apache/impala/analysis/StatementBase.java | 13 +- .../apache/impala/analysis/StmtRewriter.java | 30 +--- .../org/apache/impala/analysis/TableRef.java | 8 + .../org/apache/impala/analysis/UnionStmt.java | 16 +- .../impala/planner/HdfsPartitionPruner.java | 39 +++-- .../org/apache/impala/planner/PlanNode.java | 8 +- .../impala/rewrite/BetweenToCompoundRule.java | 63 +++++++ .../apache/impala/rewrite/ExprRewriteRule.java | 36 ++++ .../org/apache/impala/rewrite/ExprRewriter.java | 97 ++++++++++ .../impala/analysis/AnalyzeStmtsTest.java | 8 +- .../impala/analysis/AnalyzeSubqueriesTest.java | 2 +- .../impala/analysis/ExprRewriteRulesTest.java | 109 ++++++++++++ .../impala/analysis/ExprRewriterTest.java | 175 +++++++++++++++++++ .../apache/impala/common/FrontendTestBase.java | 10 +- .../queries/PlannerTest/tpcds-all.test | 12 +- .../queries/PlannerTest/tpch-all.test | 6 +- .../queries/PlannerTest/tpch-kudu.test | 2 +- .../queries/PlannerTest/tpch-nested.test | 4 +- .../queries/PlannerTest/tpch-views.test | 2 +- 30 files changed, 724 insertions(+), 214 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java index 24d1f45..b22bccb 100644 --- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java +++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java @@ -22,9 +22,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.authorization.AuthorizationChecker; import org.apache.impala.authorization.AuthorizationConfig; import org.apache.impala.authorization.AuthorizeableColumn; @@ -37,9 +34,15 @@ import org.apache.impala.catalog.ImpaladCatalog; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.InternalException; import org.apache.impala.common.Pair; +import org.apache.impala.rewrite.BetweenToCompoundRule; +import org.apache.impala.rewrite.ExprRewriteRule; +import org.apache.impala.rewrite.ExprRewriter; import org.apache.impala.thrift.TAccessEvent; import org.apache.impala.thrift.TLineageGraph; import org.apache.impala.thrift.TQueryCtx; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -52,6 +55,7 @@ public class AnalysisContext { private final ImpaladCatalog catalog_; private final TQueryCtx queryCtx_; private final AuthorizationConfig authzConfig_; + private final ExprRewriter rewriter_; // Set in analyze() private AnalysisResult analysisResult_; @@ -61,12 +65,24 @@ public class AnalysisContext { catalog_ = catalog; queryCtx_ = queryCtx; authzConfig_ = authzConfig; + List<ExprRewriteRule> rules = Lists.newArrayList(BetweenToCompoundRule.INSTANCE); + rewriter_ = new ExprRewriter(rules); + } + + /** + * C'tor with a custom ExprRewriter for testing. + */ + protected AnalysisContext(ImpaladCatalog catalog, TQueryCtx queryCtx, + AuthorizationConfig authzConfig, ExprRewriter rewriter) { + catalog_ = catalog; + queryCtx_ = queryCtx; + authzConfig_ = authzConfig; + rewriter_ = rewriter; } static public class AnalysisResult { private StatementBase stmt_; private Analyzer analyzer_; - private CreateTableStmt tmpCreateTableStmt_; public boolean isAlterTableStmt() { return stmt_ instanceof AlterTableStmt; } public boolean isAlterViewStmt() { return stmt_ instanceof AlterViewStmt; } @@ -190,10 +206,6 @@ public class AnalysisContext { return (CreateTableStmt) stmt_; } - public CreateTableStmt getTmpCreateTableStmt() { - return tmpCreateTableStmt_; - } - public CreateDbStmt getCreateDbStmt() { Preconditions.checkState(isCreateDbStmt()); return (CreateDbStmt) stmt_; @@ -311,10 +323,14 @@ public class AnalysisContext { public StatementBase getStmt() { return stmt_; } public Analyzer getAnalyzer() { return analyzer_; } public Set<TAccessEvent> getAccessEvents() { return analyzer_.getAccessEvents(); } - public boolean requiresRewrite() { + public boolean requiresSubqueryRewrite() { return analyzer_.containsSubquery() && !(stmt_ instanceof CreateViewStmt) && !(stmt_ instanceof AlterViewStmt); } + public boolean requiresExprRewrite() { + return isQueryStmt() ||isInsertStmt() || isCreateTableAsSelectStmt() + || isUpdateStmt() || isDeleteStmt(); + } public TLineageGraph getThriftLineageGraph() { return analyzer_.getThriftSerializedLineageGraph(); } @@ -352,28 +368,27 @@ public class AnalysisContext { analysisResult_.stmt_ = (StatementBase) parser.parse().value; if (analysisResult_.stmt_ == null) return; - // For CTAS, we copy the create statement in case we have to create a new CTAS - // statement after a query rewrite. - if (analysisResult_.stmt_ instanceof CreateTableAsSelectStmt) { - analysisResult_.tmpCreateTableStmt_ = - ((CreateTableAsSelectStmt)analysisResult_.stmt_).getCreateStmt().clone(); - } - analysisResult_.stmt_.analyze(analysisResult_.analyzer_); boolean isExplain = analysisResult_.isExplainStmt(); - // Check if we need to rewrite the statement. - if (analysisResult_.requiresRewrite()) { - StatementBase rewrittenStmt = StmtRewriter.rewrite(analysisResult_); - // Re-analyze the rewritten statement. - Preconditions.checkNotNull(rewrittenStmt); - analysisResult_ = new AnalysisResult(); + // Apply expr and subquery rewrites. + boolean reAnalyze = false; + if (analysisResult_.requiresExprRewrite()) { + rewriter_.reset(); + analysisResult_.stmt_.rewriteExprs(rewriter_); + reAnalyze = rewriter_.changed(); + } + if (analysisResult_.requiresSubqueryRewrite()) { + StmtRewriter.rewrite(analysisResult_); + reAnalyze = true; + } + if (reAnalyze) { analysisResult_.analyzer_ = new Analyzer(catalog_, queryCtx_, authzConfig_); - analysisResult_.stmt_ = rewrittenStmt; + analysisResult_.stmt_.reset(); analysisResult_.stmt_.analyze(analysisResult_.analyzer_); - LOG.trace("rewrittenStmt: " + rewrittenStmt.toSql()); + LOG.trace("rewrittenStmt: " + analysisResult_.stmt_.toSql()); if (isExplain) analysisResult_.stmt_.setIsExplain(); - Preconditions.checkState(!analysisResult_.requiresRewrite()); + Preconditions.checkState(!analysisResult_.requiresSubqueryRewrite()); } } catch (AnalysisException e) { // Don't wrap AnalysisExceptions in another AnalysisException http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/Analyzer.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java index b630a6e..5a34fbc 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java +++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java @@ -58,6 +58,8 @@ import org.apache.impala.common.InternalException; import org.apache.impala.common.Pair; import org.apache.impala.common.PrintUtils; import org.apache.impala.planner.PlanNode; +import org.apache.impala.rewrite.BetweenToCompoundRule; +import org.apache.impala.rewrite.ExprRewriter; import org.apache.impala.service.FeSupport; import org.apache.impala.thrift.TAccessEvent; import org.apache.impala.thrift.TCatalogObjectType; @@ -1039,6 +1041,13 @@ public class Analyzer { if ((!fromHavingClause && !hasEmptySpjResultSet_) || (fromHavingClause && !hasEmptyResultSet_)) { try { + if (conjunct instanceof BetweenPredicate) { + // Rewrite the BetweenPredicate into a CompoundPredicate so we can evaluate it + // below (BetweenPredicates are not executable). We might be in the first + // analysis pass, so the conjunct may not have been rewritten yet. + ExprRewriter rewriter = new ExprRewriter(BetweenToCompoundRule.INSTANCE); + conjunct = rewriter.rewrite(conjunct, this); + } if (!FeSupport.EvalPredicate(conjunct, globalState_.queryCtx)) { if (fromHavingClause) { hasEmptyResultSet_ = true; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/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 5e412f7..2459715 100644 --- a/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java +++ b/fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java @@ -17,38 +17,25 @@ package org.apache.impala.analysis; -import java.util.ArrayList; -import java.util.List; - import org.apache.impala.common.AnalysisException; import org.apache.impala.thrift.TExprNode; -import com.google.common.base.Preconditions; -import com.google.common.collect.Lists; /** - * Class describing between predicates. After successful analysis, we rewrite - * the between predicate to a conjunctive/disjunctive compound predicate - * to be handed to the backend. + * Class describing a BETWEEN predicate. This predicate needs to be rewritten into a + * CompoundPredicate for it to be executable, i.e., it is illegal to call toThrift() + * on this predicate because there is no BE implementation. */ public class BetweenPredicate extends Predicate { private final boolean isNotBetween_; - // After successful analysis, we rewrite this between predicate - // into a conjunctive/disjunctive compound predicate. - private CompoundPredicate rewrittenPredicate_; - - // Children of the BetweenPredicate, since this.children should hold the children - // of the rewritten predicate to make sure toThrift() picks up the right ones. - private ArrayList<Expr> originalChildren_ = Lists.newArrayList(); - // First child is the comparison expr which should be in [lowerBound, upperBound]. public BetweenPredicate(Expr compareExpr, Expr lowerBound, Expr upperBound, boolean isNotBetween) { - originalChildren_.add(compareExpr); - originalChildren_.add(lowerBound); - originalChildren_.add(upperBound); - this.isNotBetween_ = isNotBetween; + children_.add(compareExpr); + children_.add(lowerBound); + children_.add(upperBound); + isNotBetween_ = isNotBetween; } /** @@ -57,102 +44,36 @@ public class BetweenPredicate extends Predicate { protected BetweenPredicate(BetweenPredicate other) { super(other); isNotBetween_ = other.isNotBetween_; - originalChildren_ = Expr.cloneList(other.originalChildren_); - if (other.rewrittenPredicate_ != null) { - rewrittenPredicate_ = (CompoundPredicate) other.rewrittenPredicate_.clone(); - } - } - - public CompoundPredicate getRewrittenPredicate() { - Preconditions.checkState(isAnalyzed_); - return rewrittenPredicate_; } - public ArrayList<Expr> getOriginalChildren() { return originalChildren_; } @Override public void analyze(Analyzer analyzer) throws AnalysisException { if (isAnalyzed_) return; super.analyze(analyzer); - if (originalChildren_.get(0) instanceof Subquery && - (originalChildren_.get(1) instanceof Subquery || - originalChildren_.get(2) instanceof Subquery)) { + if (children_.get(0) instanceof Subquery && + (children_.get(1) instanceof Subquery || children_.get(2) instanceof Subquery)) { throw new AnalysisException("Comparison between subqueries is not " + - "supported in a between predicate: " + toSqlImpl()); + "supported in a BETWEEN predicate: " + toSqlImpl()); } - analyzer.castAllToCompatibleType(originalChildren_); - - // Rewrite between predicate into a conjunctive/disjunctive compound predicate. - if (isNotBetween_) { - // Rewrite into disjunction. - Predicate lower = new BinaryPredicate(BinaryPredicate.Operator.LT, - originalChildren_.get(0), originalChildren_.get(1)); - Predicate upper = new BinaryPredicate(BinaryPredicate.Operator.GT, - originalChildren_.get(0), originalChildren_.get(2)); - rewrittenPredicate_ = - new CompoundPredicate(CompoundPredicate.Operator.OR, lower, upper); - } else { - // Rewrite into conjunction. - Predicate lower = new BinaryPredicate(BinaryPredicate.Operator.GE, - originalChildren_.get(0), originalChildren_.get(1)); - Predicate upper = new BinaryPredicate(BinaryPredicate.Operator.LE, - originalChildren_.get(0), originalChildren_.get(2)); - rewrittenPredicate_ = - new CompoundPredicate(CompoundPredicate.Operator.AND, lower, upper); - } - - try { - rewrittenPredicate_.analyze(analyzer); - fn_ = rewrittenPredicate_.fn_; - } catch (AnalysisException e) { - // We should have already guaranteed that analysis will succeed. - Preconditions.checkState(false, "Analysis failed in rewritten between predicate"); - } - - // Make sure toThrift() picks up the children of the rewritten predicate. - children_ = rewrittenPredicate_.getChildren(); - // Since the only child is a CompoundPredicate expressing the comparison, - // the cost of the comparison is fully captured by the children's cost. - evalCost_ = getChildCosts(); + analyzer.castAllToCompatibleType(children_); isAnalyzed_ = true; } - @Override - public List<Expr> getConjuncts() { - return rewrittenPredicate_.getConjuncts(); - } + public boolean isNotBetween() { return isNotBetween_; } @Override protected void toThrift(TExprNode msg) { - rewrittenPredicate_.toThrift(msg); + throw new IllegalStateException( + "BetweenPredicate needs to be rewritten into a CompoundPredicate."); } @Override public String toSqlImpl() { String notStr = (isNotBetween_) ? "NOT " : ""; - return originalChildren_.get(0).toSql() + " " + notStr + "BETWEEN " + - originalChildren_.get(1).toSql() + " AND " + originalChildren_.get(2).toSql(); - } - - /** - * Also substitute the exprs in originalChildren when cloning. - */ - @Override - protected Expr substituteImpl(ExprSubstitutionMap smap, Analyzer analyzer) - throws AnalysisException { - BetweenPredicate clone = (BetweenPredicate) super.substituteImpl(smap, analyzer); - Preconditions.checkNotNull(clone); - clone.originalChildren_ = - Expr.substituteList(originalChildren_, smap, analyzer, false); - return clone; + return children_.get(0).toSql() + " " + notStr + "BETWEEN " + + children_.get(1).toSql() + " AND " + children_.get(2).toSql(); } @Override public Expr clone() { return new BetweenPredicate(this); } - - @Override - public Expr reset() { - super.reset(); - originalChildren_ = Expr.resetList(originalChildren_); - return this; - } } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java index 1931930..54b8557 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java +++ b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java @@ -21,12 +21,12 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.common.AnalysisException; import org.apache.impala.thrift.TCreateOrAlterViewParams; import org.apache.impala.thrift.TTableName; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -156,7 +156,7 @@ public abstract class CreateOrAlterViewStmtBase extends StatementBase { } /** - * Computes the column lineage graph for a create/alter view statetement. + * Computes the column lineage graph for a create/alter view statement. */ protected void computeLineageGraph(Analyzer analyzer) { ColumnLineageGraph graph = analyzer.getColumnLineageGraph(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java index f7f8417..2f5f166 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java @@ -17,8 +17,8 @@ package org.apache.impala.analysis; -import java.util.List; import java.util.EnumSet; +import java.util.List; import org.apache.impala.authorization.Privilege; import org.apache.impala.catalog.Db; @@ -27,8 +27,10 @@ import org.apache.impala.catalog.KuduTable; import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient; import org.apache.impala.catalog.Table; import org.apache.impala.common.AnalysisException; +import org.apache.impala.rewrite.ExprRewriter; import org.apache.impala.service.CatalogOpExecutor; import org.apache.impala.thrift.THdfsFileFormat; + import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -209,6 +211,12 @@ public class CreateTableAsSelectStmt extends StatementBase { } @Override + public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException { + Preconditions.checkState(isAnalyzed()); + insertStmt_.rewriteExprs(rewriter); + } + + @Override public void reset() { super.reset(); insertStmt_.reset(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java b/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java index 94f5e76..3ee2070 100644 --- a/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java +++ b/fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java @@ -21,14 +21,15 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.catalog.ColumnStats; import org.apache.impala.catalog.StructField; import org.apache.impala.catalog.StructType; import org.apache.impala.catalog.View; import org.apache.impala.common.AnalysisException; +import org.apache.impala.rewrite.ExprRewriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -262,6 +263,13 @@ public class InlineViewRef extends TableRef { } @Override + public void rewriteExprs(ExprRewriter rewriter, Analyzer analyzer) + throws AnalysisException { + super.rewriteExprs(rewriter, analyzer); + queryStmt_.rewriteExprs(rewriter); + } + + @Override public List<TupleId> getMaterializedTupleIds() { Preconditions.checkState(isAnalyzed_); Preconditions.checkState(materializedTupleIds_.size() > 0); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java index e95516e..5e457a3 100644 --- a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java @@ -22,11 +22,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -import org.apache.impala.planner.TableSink; -import com.google.common.collect.ImmutableList; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.authorization.Privilege; import org.apache.impala.authorization.PrivilegeRequestBuilder; import org.apache.impala.catalog.Column; @@ -39,8 +34,14 @@ import org.apache.impala.catalog.View; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.FileSystemUtil; import org.apache.impala.planner.DataSink; +import org.apache.impala.planner.TableSink; +import org.apache.impala.rewrite.ExprRewriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Sets; @@ -659,6 +660,12 @@ public class InsertStmt extends StatementBase { } } + @Override + public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException { + Preconditions.checkState(isAnalyzed()); + queryStmt_.rewriteExprs(rewriter); + } + public List<String> getPlanHints() { return planHints_; } public TableName getTargetTableName() { return targetTableName_; } public Table getTargetTable() { return table_; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java index be22f02..00f99a5 100644 --- a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java @@ -17,6 +17,8 @@ package org.apache.impala.analysis; +import static java.lang.String.format; + import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -30,14 +32,14 @@ import org.apache.impala.catalog.Table; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.Pair; import org.apache.impala.planner.DataSink; +import org.apache.impala.rewrite.ExprRewriter; +import org.slf4j.LoggerFactory; + import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; -import org.slf4j.LoggerFactory; - -import static java.lang.String.format; /** * Abstract super class for statements that modify existing data like @@ -286,8 +288,18 @@ public abstract class ModifyStmt extends StatementBase { } } + @Override + public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException { + Preconditions.checkState(isAnalyzed()); + for (Pair<SlotRef, Expr> assignment: assignments_) { + assignment.second = rewriter.rewrite(assignment.second, analyzer_); + } + sourceStmt_.rewriteExprs(rewriter); + } + public QueryStmt getQueryStmt() { return sourceStmt_; } public abstract DataSink createDataSink(); + @Override public abstract String toSql(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/SelectList.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectList.java b/fe/src/main/java/org/apache/impala/analysis/SelectList.java index 4297f94..4c504bf 100644 --- a/fe/src/main/java/org/apache/impala/analysis/SelectList.java +++ b/fe/src/main/java/org/apache/impala/analysis/SelectList.java @@ -19,6 +19,9 @@ package org.apache.impala.analysis; import java.util.List; +import org.apache.impala.common.AnalysisException; +import org.apache.impala.rewrite.ExprRewriter; + import com.google.common.collect.Lists; /** @@ -83,6 +86,14 @@ public class SelectList { } } + public void rewriteExprs(ExprRewriter rewriter, Analyzer analyzer) + throws AnalysisException { + for (SelectListItem item: items_) { + if (item.isStar()) continue; + item.setExpr(rewriter.rewrite(item.getExpr(), analyzer)); + } + } + @Override public SelectList clone() { return new SelectList(this); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java b/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java index 80aee25..b47e59f 100644 --- a/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java +++ b/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java @@ -23,7 +23,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; class SelectListItem { - private final Expr expr_; + private Expr expr_; private String alias_; // for "[path.]*" (excludes trailing '*') @@ -52,6 +52,7 @@ class SelectListItem { } public Expr getExpr() { return expr_; } + public void setExpr(Expr expr) { expr_ = expr; } public boolean isStar() { return isStar_; } public String getAlias() { return alias_; } public void setAlias(String alias) { alias_ = alias; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java index 9585f47..b112083 100644 --- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java @@ -22,9 +22,6 @@ import java.util.Collections; import java.util.List; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.analysis.Path.PathType; import org.apache.impala.catalog.Column; import org.apache.impala.catalog.StructField; @@ -35,6 +32,10 @@ import org.apache.impala.common.AnalysisException; import org.apache.impala.common.ColumnAliasGenerator; import org.apache.impala.common.TableAliasGenerator; import org.apache.impala.common.TreeNode; +import org.apache.impala.rewrite.ExprRewriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Preconditions; import com.google.common.base.Predicates; import com.google.common.collect.Iterables; @@ -56,7 +57,7 @@ public class SelectStmt extends QueryStmt { protected final FromClause fromClause_; protected Expr whereClause_; protected ArrayList<Expr> groupingExprs_; - protected final Expr havingClause_; // original having clause + protected Expr havingClause_; // original having clause // havingClause with aliases and agg output resolved private Expr havingPred_; @@ -863,6 +864,29 @@ public class SelectStmt extends QueryStmt { } } + @Override + public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException { + Preconditions.checkState(isAnalyzed()); + selectList_.rewriteExprs(rewriter, analyzer_); + for (TableRef ref: fromClause_.getTableRefs()) ref.rewriteExprs(rewriter, analyzer_); + if (whereClause_ != null) { + whereClause_ = rewriter.rewrite(whereClause_, analyzer_); + // Also rewrite exprs in the statements of subqueries. + List<Subquery> subqueryExprs = Lists.newArrayList(); + whereClause_.collect(Subquery.class, subqueryExprs); + for (Subquery s: subqueryExprs) s.getStatement().rewriteExprs(rewriter); + } + if (havingClause_ != null) { + havingClause_ = rewriter.rewrite(havingClause_, analyzer_); + } + if (groupingExprs_ != null) rewriter.applyList(groupingExprs_, analyzer_); + if (orderByElements_ != null) { + for (OrderByElement orderByElem: orderByElements_) { + orderByElem.setExpr(rewriter.rewrite(orderByElem.getExpr(), analyzer_)); + } + } + } + /** * Returns the SQL string corresponding to this SelectStmt. */ http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/StatementBase.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/StatementBase.java b/fe/src/main/java/org/apache/impala/analysis/StatementBase.java index 9faaddd..b4e3155 100644 --- a/fe/src/main/java/org/apache/impala/analysis/StatementBase.java +++ b/fe/src/main/java/org/apache/impala/analysis/StatementBase.java @@ -18,10 +18,10 @@ package org.apache.impala.analysis; import org.apache.commons.lang.NotImplementedException; - import org.apache.impala.catalog.Column; import org.apache.impala.catalog.Type; import org.apache.impala.common.AnalysisException; +import org.apache.impala.rewrite.ExprRewriter; /** * Base class for all Impala SQL statements. @@ -63,6 +63,17 @@ abstract class StatementBase implements ParseNode { analyzer_ = analyzer; } + /** + * Uses the given 'rewriter' to transform all Exprs in this statement according + * to the rules specified in the 'rewriter'. Replaces the original Exprs with the + * transformed ones in-place. Subclasses that have Exprs to be rewritten must + * override this method. Valid to call after analyze(). + */ + public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException { + throw new IllegalStateException( + "rewriteExprs() not implemented for this stmt: " + getClass().getSimpleName()); + } + public Analyzer getAnalyzer() { return analyzer_; } public boolean isAnalyzed() { return analyzer_ != null; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java b/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java index bc18dfd..eccb136 100644 --- a/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java +++ b/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java @@ -20,12 +20,12 @@ package org.apache.impala.analysis; import java.util.ArrayList; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.analysis.AnalysisContext.AnalysisResult; import org.apache.impala.analysis.UnionStmt.UnionOperand; import org.apache.impala.common.AnalysisException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Preconditions; import com.google.common.base.Predicates; import com.google.common.collect.Iterables; @@ -41,10 +41,10 @@ public class StmtRewriter { private final static Logger LOG = LoggerFactory.getLogger(StmtRewriter.class); /** - * Rewrite the statement of an analysis result. The unanalyzed rewritten - * statement is returned. + * Rewrite the statement of an analysis result in-place. Assumes that BetweenPredicates + * have already been rewritten. */ - public static StatementBase rewrite(AnalysisResult analysisResult) + public static void rewrite(AnalysisResult analysisResult) throws AnalysisException { // Analyzed stmt that contains a query statement with subqueries to be rewritten. StatementBase stmt = analysisResult.getStmt(); @@ -66,8 +66,6 @@ public class StmtRewriter { stmt.toSql()); } rewriteQueryStatement(queryStmt, queryStmt.getAnalyzer()); - stmt.reset(); - return stmt; } /** @@ -204,8 +202,6 @@ public class StmtRewriter { int numTableRefs = stmt.fromClause_.size(); ArrayList<Expr> exprsWithSubqueries = Lists.newArrayList(); ExprSubstitutionMap smap = new ExprSubstitutionMap(); - // Replace all BetweenPredicates with their equivalent compound predicates. - stmt.whereClause_ = rewriteBetweenPredicates(stmt.whereClause_); // Check if all the conjuncts in the WHERE clause that contain subqueries // can currently be rewritten as a join. for (Expr conjunct: stmt.whereClause_.getConjuncts()) { @@ -276,20 +272,6 @@ public class StmtRewriter { } /** - * Replace all BetweenPredicates with their equivalent compound predicates from the - * expr tree rooted at 'expr'. The modified expr tree is returned. - */ - private static Expr rewriteBetweenPredicates(Expr expr) { - if (expr instanceof BetweenPredicate) { - return ((BetweenPredicate)expr).getRewrittenPredicate(); - } - for (int i = 0; i < expr.getChildren().size(); ++i) { - expr.setChild(i, rewriteBetweenPredicates(expr.getChild(i))); - } - return expr; - } - - /** * Modifies in place an expr that contains a subquery by rewriting its * subquery stmt. The modified analyzed expr is returned. */ http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/analysis/TableRef.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/TableRef.java b/fe/src/main/java/org/apache/impala/analysis/TableRef.java index c276dba..6403091 100644 --- a/fe/src/main/java/org/apache/impala/analysis/TableRef.java +++ b/fe/src/main/java/org/apache/impala/analysis/TableRef.java @@ -26,7 +26,9 @@ import org.apache.impala.catalog.HdfsTable; import org.apache.impala.catalog.Table; import org.apache.impala.common.AnalysisException; import org.apache.impala.planner.JoinNode.DistributionMode; +import org.apache.impala.rewrite.ExprRewriter; import org.apache.impala.thrift.TReplicaPreference; + import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -509,6 +511,12 @@ public class TableRef implements ParseNode { } } + public void rewriteExprs(ExprRewriter rewriter, Analyzer analyzer) + throws AnalysisException { + Preconditions.checkState(isAnalyzed_); + if (onClause_ != null) onClause_ = rewriter.rewrite(onClause_, analyzer); + } + protected String tableRefToSql() { String aliasSql = null; String alias = getExplicitAlias(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/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 fffb72c..674e399 100644 --- a/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java @@ -22,6 +22,7 @@ import java.util.List; import org.apache.impala.catalog.ColumnStats; import org.apache.impala.common.AnalysisException; +import org.apache.impala.rewrite.ExprRewriter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -198,6 +199,7 @@ public class UnionStmt extends QueryStmt { public boolean hasAllOps() { return !allOperands_.isEmpty(); } public AggregateInfo getDistinctAggInfo() { return distinctAggInfo_; } public boolean hasAnalyticExprs() { return hasAnalyticExprs_; } + public TupleId getTupleId() { return tupleId_; } public void removeAllOperands() { operands_.removeAll(allOperands_); @@ -519,7 +521,15 @@ public class UnionStmt extends QueryStmt { baseTblResultExprs_ = resultExprs_; } - public TupleId getTupleId() { return tupleId_; } + @Override + public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException { + for (UnionOperand op: operands_) op.getQueryStmt().rewriteExprs(rewriter); + if (orderByElements_ != null) { + for (OrderByElement orderByElem: orderByElements_) { + orderByElem.setExpr(rewriter.rewrite(orderByElem.getExpr(), analyzer_)); + } + } + } @Override public void getMaterializedTupleIds(ArrayList<TupleId> tupleIdList) { @@ -533,9 +543,7 @@ public class UnionStmt extends QueryStmt { @Override public void collectTableRefs(List<TableRef> tblRefs) { - for (UnionOperand op: operands_) { - op.getQueryStmt().collectTableRefs(tblRefs); - } + for (UnionOperand op: operands_) op.getQueryStmt().collectTableRefs(tblRefs); } @Override http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java b/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java index 5efd474..10b6539 100644 --- a/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java +++ b/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java @@ -26,9 +26,6 @@ import java.util.NavigableMap; import java.util.Set; import java.util.TreeMap; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.BetweenPredicate; import org.apache.impala.analysis.BinaryPredicate; @@ -47,6 +44,11 @@ import org.apache.impala.catalog.HdfsPartition; import org.apache.impala.catalog.HdfsTable; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.InternalException; +import org.apache.impala.rewrite.BetweenToCompoundRule; +import org.apache.impala.rewrite.ExprRewriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Preconditions; import com.google.common.base.Predicates; import com.google.common.collect.Lists; @@ -73,7 +75,12 @@ public class HdfsPartitionPruner { private final HdfsTable tbl_; - private List<SlotId> partitionSlots_ = Lists.newArrayList(); + private final List<SlotId> partitionSlots_ = Lists.newArrayList(); + + // For converting BetweenPredicates to CompoundPredicates so they can be + // executed in the BE. + private final ExprRewriter exprRewriter_ = + new ExprRewriter(BetweenToCompoundRule.INSTANCE); public HdfsPartitionPruner(TupleDescriptor tupleDesc) { Preconditions.checkState(tupleDesc.getTable() instanceof HdfsTable); @@ -90,10 +97,10 @@ public class HdfsPartitionPruner { /** * Return a list of partitions left after applying the conjuncts. Please note - * that conjunts used for filtering will be removed from the list 'conjuncts'. + * that conjuncts used for filtering will be removed from the list 'conjuncts'. */ public List<HdfsPartition> prunePartitions(Analyzer analyzer, List<Expr> conjuncts) - throws InternalException { + throws InternalException, AnalysisException { // Start with creating a collection of partition filters for the applicable conjuncts. List<HdfsPartitionFilter> partitionFilters = Lists.newArrayList(); // Conjuncts that can be evaluated from the partition key values. @@ -109,15 +116,14 @@ public class HdfsPartitionPruner { Expr conjunct = it.next(); if (conjunct.isBoundBySlotIds(partitionSlots_)) { // Check if the conjunct can be evaluated from the partition metadata. - // canEvalUsingPartitionMd() operates on a cloned conjunct which may get - // modified if it contains constant expressions. If the cloned conjunct - // cannot be evaluated from the partition metadata, the original unmodified - // conjuct is evaluated in the BE. - Expr clonedConjunct = conjunct.clone(); + // Use a cloned conjunct to rewrite BetweenPredicates and allow + // canEvalUsingPartitionMd() to fold constant expressions without modifying + // the original expr. + Expr clonedConjunct = exprRewriter_.rewrite(conjunct.clone(), analyzer); if (canEvalUsingPartitionMd(clonedConjunct, analyzer)) { simpleFilterConjuncts.add(Expr.pushNegationToOperands(clonedConjunct)); } else { - partitionFilters.add(new HdfsPartitionFilter(conjunct, tbl_, analyzer)); + partitionFilters.add(new HdfsPartitionFilter(clonedConjunct, tbl_, analyzer)); } it.remove(); } @@ -169,6 +175,7 @@ public class HdfsPartitionPruner { */ private boolean canEvalUsingPartitionMd(Expr expr, Analyzer analyzer) { Preconditions.checkNotNull(expr); + Preconditions.checkState(!(expr instanceof BetweenPredicate)); if (expr instanceof BinaryPredicate) { // Evaluate any constant expression in the BE try { @@ -208,9 +215,6 @@ public class HdfsPartitionPruner { if (!(expr.getChild(i).isLiteral())) return false; } return true; - } else if (expr instanceof BetweenPredicate) { - return canEvalUsingPartitionMd(((BetweenPredicate) expr).getRewrittenPredicate(), - analyzer); } return false; } @@ -407,10 +411,11 @@ public class HdfsPartitionPruner { * key values; return the matching partition ids. An empty set is returned * if there are no matching partitions. This function can evaluate the following * types of predicates: BinaryPredicate, CompoundPredicate, IsNullPredicate, - * InPredicate, and BetweenPredicate. + * InPredicate. */ private HashSet<Long> evalSlotBindingFilter(Expr expr) { Preconditions.checkNotNull(expr); + Preconditions.checkState(!(expr instanceof BetweenPredicate)); if (expr instanceof BinaryPredicate) { return evalBinaryPredicate(expr); } else if (expr instanceof CompoundPredicate) { @@ -430,8 +435,6 @@ public class HdfsPartitionPruner { return evalInPredicate(expr); } else if (expr instanceof IsNullPredicate) { return evalIsNullPredicate(expr); - } else if (expr instanceof BetweenPredicate) { - return evalSlotBindingFilter(((BetweenPredicate) expr).getRewrittenPredicate()); } return null; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/planner/PlanNode.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/planner/PlanNode.java b/fe/src/main/java/org/apache/impala/planner/PlanNode.java index 4686e2c..141464e 100644 --- a/fe/src/main/java/org/apache/impala/planner/PlanNode.java +++ b/fe/src/main/java/org/apache/impala/planner/PlanNode.java @@ -23,9 +23,6 @@ import java.util.Collections; import java.util.List; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.Expr; import org.apache.impala.analysis.ExprId; @@ -41,6 +38,9 @@ import org.apache.impala.thrift.TExplainLevel; import org.apache.impala.thrift.TPlan; import org.apache.impala.thrift.TPlanNode; import org.apache.impala.thrift.TQueryOptions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -669,7 +669,7 @@ abstract public class PlanNode extends TreeNode<PlanNode> { int numWithoutSel = 0; List<T> remaining = Lists.newArrayListWithCapacity(conjuncts.size()); for (T e : conjuncts) { - Preconditions.checkState(e.hasCost()); + Preconditions.checkState(e.hasCost(), e.toSql()); totalCost += e.getCost(); remaining.add(e); if (!e.hasSelectivity()) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java b/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java new file mode 100644 index 0000000..296780d --- /dev/null +++ b/fe/src/main/java/org/apache/impala/rewrite/BetweenToCompoundRule.java @@ -0,0 +1,63 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.impala.rewrite; + +import org.apache.impala.analysis.Analyzer; +import org.apache.impala.analysis.BetweenPredicate; +import org.apache.impala.analysis.BinaryPredicate; +import org.apache.impala.analysis.CompoundPredicate; +import org.apache.impala.analysis.Expr; +import org.apache.impala.analysis.Predicate; +import org.apache.impala.common.AnalysisException; + +/** + * Rewrites BetweenPredicates into an equivalent conjunctive/disjunctive + * CompoundPredicate. + * Examples: + * A BETWEEN X AND Y ==> A >= X AND A <= Y + * A NOT BETWEEN X AND Y ==> A < X OR A > Y + */ +public class BetweenToCompoundRule implements ExprRewriteRule { + public static ExprRewriteRule INSTANCE = new BetweenToCompoundRule(); + + @Override + public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException { + if (!(expr instanceof BetweenPredicate)) return expr; + BetweenPredicate bp = (BetweenPredicate) expr; + Expr result = null; + if (bp.isNotBetween()) { + // Rewrite into disjunction. + Predicate lower = new BinaryPredicate(BinaryPredicate.Operator.LT, + bp.getChild(0), bp.getChild(1)); + Predicate upper = new BinaryPredicate(BinaryPredicate.Operator.GT, + bp.getChild(0), bp.getChild(2)); + result = new CompoundPredicate(CompoundPredicate.Operator.OR, lower, upper); + } else { + // Rewrite into conjunction. + Predicate lower = new BinaryPredicate(BinaryPredicate.Operator.GE, + bp.getChild(0), bp.getChild(1)); + Predicate upper = new BinaryPredicate(BinaryPredicate.Operator.LE, + bp.getChild(0), bp.getChild(2)); + result = new CompoundPredicate(CompoundPredicate.Operator.AND, lower, upper); + } + result.analyze(analyzer); + return result; + } + + private BetweenToCompoundRule() {} +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java b/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java new file mode 100644 index 0000000..48054a8 --- /dev/null +++ b/fe/src/main/java/org/apache/impala/rewrite/ExprRewriteRule.java @@ -0,0 +1,36 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.impala.rewrite; + +import org.apache.impala.analysis.Analyzer; +import org.apache.impala.analysis.Expr; +import org.apache.impala.common.AnalysisException; + +/** + * Base class for all Expr rewrite rules. A rule is free to modify Exprs in place, + * but must return a different Expr object if any modifications were made. + */ +public interface ExprRewriteRule { + /** + * Applies this rewrite rule to the given analyzed Expr. Returns the transformed and + * analyzed Expr or the original unmodified Expr if no changes were made. If any + * changes were made, the transformed Expr is guaranteed to be a different Expr object, + * so callers can rely on object reference comparison for change detection. + */ + public abstract Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException; +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java b/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java new file mode 100644 index 0000000..906fc0e --- /dev/null +++ b/fe/src/main/java/org/apache/impala/rewrite/ExprRewriter.java @@ -0,0 +1,97 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.impala.rewrite; + +import java.util.List; + +import org.apache.impala.analysis.Analyzer; +import org.apache.impala.analysis.Expr; +import org.apache.impala.common.AnalysisException; +import org.apache.kudu.client.shaded.com.google.common.collect.Lists; + +/** + * Helper class that drives the transformation of Exprs according to a given list of + * ExprRewriteRules. The rules are applied as follows: + * - a single rule is applied repeatedly to the Expr and all its children in a bottom-up + * fashion until there are no more changes + * - the rule list is applied repeatedly until no rule has made any changes + * - the rules are applied in the order they appear in the rule list + * Keeps track of how many transformations were applied. + */ +public class ExprRewriter { + private int numChanges_ = 0; + private final List<ExprRewriteRule> rules_; + + public ExprRewriter(List<ExprRewriteRule> rules) { + rules_ = rules; + } + + public ExprRewriter(ExprRewriteRule rule) { + rules_ = Lists.newArrayList(rule); + } + + public Expr rewrite(Expr expr, Analyzer analyzer) throws AnalysisException { + // Keep applying the rule list until no rule has made any changes. + int oldNumChanges; + Expr rewrittenExpr = expr; + do { + oldNumChanges = numChanges_; + for (ExprRewriteRule rule: rules_) { + rewrittenExpr = applyRuleRepeatedly(rewrittenExpr, rule, analyzer); + } + } while (oldNumChanges != numChanges_); + return rewrittenExpr; + } + + /** + * Applies 'rule' on the Expr tree rooted at 'expr' until there are no more changes. + * Returns the transformed Expr or 'expr' if there were no changes. + */ + private Expr applyRuleRepeatedly(Expr expr, ExprRewriteRule rule, Analyzer analyzer) + throws AnalysisException { + int oldNumChanges; + Expr rewrittenExpr = expr; + do { + oldNumChanges = numChanges_; + rewrittenExpr = applyRuleBottomUp(rewrittenExpr, rule, analyzer); + } while (oldNumChanges != numChanges_); + return rewrittenExpr; + } + + /** + * Applies 'rule' on 'expr' and all its children in a bottom-up fashion. + * Returns the transformed Expr or 'expr' if there were no changes. + */ + private Expr applyRuleBottomUp(Expr expr, ExprRewriteRule rule, Analyzer analyzer) + throws AnalysisException { + for (int i = 0; i < expr.getChildren().size(); ++i) { + expr.setChild(i, applyRuleBottomUp(expr.getChild(i), rule, analyzer)); + } + Expr rewrittenExpr = rule.apply(expr, analyzer); + if (rewrittenExpr != expr) ++numChanges_; + return rewrittenExpr; + } + + public void applyList(List<Expr> exprs, Analyzer analyzer) throws AnalysisException { + for (int i = 0; i < exprs.size(); ++i) exprs.set(i, rewrite(exprs.get(i), analyzer)); + } + + public void reset() { numChanges_ = 0; } + public boolean changed() { return numChanges_ > 0; } + public int getNumChanges() { return numChanges_; } +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java index 31c0fdc..cbd2d07 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java @@ -23,13 +23,13 @@ import static org.junit.Assert.fail; import java.lang.reflect.Field; import java.util.List; -import org.junit.Assert; -import org.junit.Test; - import org.apache.impala.catalog.PrimitiveType; import org.apache.impala.catalog.ScalarType; import org.apache.impala.catalog.Type; import org.apache.impala.common.AnalysisException; +import org.junit.Assert; +import org.junit.Test; + import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; @@ -2769,7 +2769,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest { "from with_1) select 1 as int_col_1 from with_4) as t1) select 1 as " + "int_col_1 from with_3) select 1 as int_col_1 from with_2"); - // WITH clasue with a between predicate + // WITH clause with a between predicate AnalyzesOk("with with_1 as (select int_col from functional.alltypestiny " + "where int_col between 0 and 10) select * from with_1"); // WITH clause with a between predicate in the select list http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java index a37ba72..fb2d63a 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java @@ -1276,7 +1276,7 @@ public class AnalyzeSubqueriesTest extends AnalyzerTest { AnalysisError("select * from functional.alltypestiny where (select min(id) " + "from functional.alltypes) between 1 and (select max(id) from " + "functional.alltypes)", "Comparison between subqueries is not supported " + - "in a between predicate: (SELECT min(id) FROM functional.alltypes) BETWEEN " + + "in a BETWEEN predicate: (SELECT min(id) FROM functional.alltypes) BETWEEN " + "1 AND (SELECT max(id) FROM functional.alltypes)"); AnalyzesOk("select * from functional.alltypestiny where " + "int_col between 0 and 10 and exists (select 1)"); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java new file mode 100644 index 0000000..d3d52b4 --- /dev/null +++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java @@ -0,0 +1,109 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.impala.analysis; + +import java.util.List; + +import org.apache.impala.catalog.Catalog; +import org.apache.impala.common.AnalysisException; +import org.apache.impala.common.FrontendTestBase; +import org.apache.impala.rewrite.BetweenToCompoundRule; +import org.apache.impala.rewrite.ExprRewriteRule; +import org.apache.impala.rewrite.ExprRewriter; +import org.junit.Assert; +import org.junit.Test; + +import com.google.common.collect.Lists; + +/** + * Tests ExprRewriteRules. + */ +public class ExprRewriteRulesTest extends FrontendTestBase { + + public Expr RewritesOk(String expr, ExprRewriteRule rule, String expectedExpr) + throws AnalysisException { + String stmtStr = "select " + expr + " from functional.alltypes"; + SelectStmt stmt = (SelectStmt) ParsesOk(stmtStr); + Analyzer analyzer = createAnalyzer(Catalog.DEFAULT_DB); + stmt.analyze(analyzer); + Expr origExpr = stmt.getResultExprs().get(0); + String origSql = origExpr.toSql(); + // Create a rewriter with only a single rule. + List<ExprRewriteRule> rules = Lists.newArrayList(); + rules.add(rule); + ExprRewriter rewriter = new ExprRewriter(rules); + Expr rewrittenExpr = rewriter.rewrite(origExpr, analyzer); + String rewrittenSql = rewrittenExpr.toSql(); + boolean expectChange = expectedExpr != null; + if (expectedExpr != null) { + assertEquals(expectedExpr, rewrittenSql); + } else { + assertEquals(origSql, rewrittenSql); + } + Assert.assertEquals(expectChange, rewriter.changed()); + return rewrittenExpr; + } + + /** + * Helper for prettier error messages than what JUnit.Assert provides. + */ + private void assertEquals(String expected, String actual) { + if (!actual.equals(expected)) { + Assert.fail(String.format("\nActual: %s\nExpected: %s\n", actual, expected)); + } + } + + @Test + public void TestBetweenToCompoundRule() throws AnalysisException { + ExprRewriteRule rule = BetweenToCompoundRule.INSTANCE; + + // Basic BETWEEN predicates. + RewritesOk("int_col between float_col and double_col", rule, + "int_col >= float_col AND int_col <= double_col"); + RewritesOk("int_col not between float_col and double_col", rule, + "int_col < float_col OR int_col > double_col"); + RewritesOk("50.0 between null and 5000", rule, + "50.0 >= NULL AND 50.0 <= 5000"); + // Basic NOT BETWEEN predicates. + RewritesOk("int_col between 10 and 20", rule, + "int_col >= 10 AND int_col <= 20"); + RewritesOk("int_col not between 10 and 20", rule, + "int_col < 10 OR int_col > 20"); + RewritesOk("50.0 not between null and 5000", rule, + "50.0 < NULL OR 50.0 > 5000"); + + // Nested BETWEEN predicates. + RewritesOk( + "int_col between if(tinyint_col between 1 and 2, 10, 20) " + + "and cast(smallint_col between 1 and 2 as int)", rule, + "int_col >= if(tinyint_col >= 1 AND tinyint_col <= 2, 10, 20) " + + "AND int_col <= CAST(smallint_col >= 1 AND smallint_col <= 2 AS INT)"); + // Nested NOT BETWEEN predicates. + RewritesOk( + "int_col not between if(tinyint_col not between 1 and 2, 10, 20) " + + "and cast(smallint_col not between 1 and 2 as int)", rule, + "int_col < if(tinyint_col < 1 OR tinyint_col > 2, 10, 20) " + + "OR int_col > CAST(smallint_col < 1 OR smallint_col > 2 AS INT)"); + // Mixed nested BETWEEN and NOT BETWEEN predicates. + RewritesOk( + "int_col between if(tinyint_col between 1 and 2, 10, 20) " + + "and cast(smallint_col not between 1 and 2 as int)", rule, + "int_col >= if(tinyint_col >= 1 AND tinyint_col <= 2, 10, 20) " + + "AND int_col <= CAST(smallint_col < 1 OR smallint_col > 2 AS INT)"); + } +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java new file mode 100644 index 0000000..310b7e4 --- /dev/null +++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java @@ -0,0 +1,175 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.impala.analysis; + +import org.apache.impala.authorization.AuthorizationConfig; +import org.apache.impala.catalog.Catalog; +import org.apache.impala.common.AnalysisException; +import org.apache.impala.common.RuntimeEnv; +import org.apache.impala.rewrite.ExprRewriteRule; +import org.apache.impala.rewrite.ExprRewriter; +import org.apache.impala.testutil.TestUtils; +import org.junit.Assert; +import org.junit.Test; + +import com.google.common.base.Preconditions; + +/** + * Tests that the ExprRewriter framework covers all clauses as well as nested statements. + * Does not test specific rewrite rules. + */ +public class ExprRewriterTest extends AnalyzerTest { + + /** + * Replaces any Expr that does not contain a Subquery with a TRUE BoolLiteral. + */ + static class ExprToBoolRule implements ExprRewriteRule { + public static ExprToBoolRule INSTANCE = new ExprToBoolRule(); + + @Override + public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException { + if (expr.contains(Subquery.class)) return expr; + if (Predicate.IS_TRUE_LITERAL.apply(expr)) return expr; + return new BoolLiteral(true); + } + + private ExprToBoolRule() {} + } + + /** + * Replaces a TRUE BoolLiteral with a FALSE BoolLiteral. + */ + static class TrueToFalseRule implements ExprRewriteRule { + public static TrueToFalseRule INSTANCE = new TrueToFalseRule(); + + @Override + public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException { + if (Predicate.IS_TRUE_LITERAL.apply(expr)) return new BoolLiteral(false); + return expr; + } + private TrueToFalseRule() {} + } + + private final ExprRewriter exprToTrue_ = new ExprRewriter(ExprToBoolRule.INSTANCE); + private final ExprRewriter trueToFalse_ = new ExprRewriter(TrueToFalseRule.INSTANCE); + + /** + * Analyzes 'stmt' and rewrites Exprs with 'exprToTrue_' and validates the following: + * 1. The actual number of changed Exprs should be equal to 'expectedNumChanges'. + * 2. Checks that the Exprs were actually rewritten by doing another round of + * rewriting using 'trueToFalse_'. The expected number of changes for this + * second rewriting is 'expectedNumExprTrees'. + * Does not use an AnalysisContext to avoid rewriting subqueries which might alter the + * number of expressions and complicate validation. + */ + public void RewritesOk(String stmt, int expectedNumChanges, + int expectedNumExprTrees) throws AnalysisException { + StatementBase parsedStmt = (StatementBase) ParsesOk(stmt); + parsedStmt.analyze(createAnalyzer(Catalog.DEFAULT_DB)); + + exprToTrue_.reset(); + parsedStmt.rewriteExprs(exprToTrue_); + Assert.assertEquals(expectedNumChanges, exprToTrue_.getNumChanges()); + + // Verify that the Exprs were actually replaced. + trueToFalse_.reset(); + parsedStmt.rewriteExprs(trueToFalse_); + Assert.assertEquals(expectedNumExprTrees, trueToFalse_.getNumChanges()); + + // Make sure the stmt can be successfully re-analyzed. + parsedStmt.reset(); + parsedStmt.analyze(createAnalyzer(Catalog.DEFAULT_DB)); + } + + /** + * Asserts that no rewrites are performed on the given stmt. + */ + public void CheckNoRewrite(String stmt) throws AnalysisException { + exprToTrue_.reset(); + Analyzer analyzer = createAnalyzer(Catalog.DEFAULT_DB); + AnalysisContext analysisCtx = new AnalysisContext(catalog_, + TestUtils.createQueryContext(Catalog.DEFAULT_DB, + System.getProperty("user.name")), + AuthorizationConfig.createAuthDisabledConfig(), exprToTrue_); + analysisCtx.analyze(stmt, analyzer); + Preconditions.checkNotNull(analysisCtx.getAnalysisResult().getStmt()); + Assert.assertEquals(0, exprToTrue_.getNumChanges()); + } + + // Select statement with all clauses that has 11 rewritable Expr trees. + // We expect a total of 23 exprs to be changed. + private final String stmt_ = + "select a.int_col a, 10 b, 20.2 c, count(b.int_col) cnt from " + + "functional.alltypes a join functional.alltypes b on (a.id = b.id)" + + "where b.float_col > 1 and b.double_col > 2 " + + "group by 1, a.string_col " + + "having count(b.int_col) < 3 " + + "order by a.int_col, 4 limit 10"; + + @Test + public void TestQueryStmts() throws AnalysisException { + RewritesOk(stmt_, 23, 11); + // Test rewriting in inline views. The view stmt is the same as the query above + // but with an order by + limit. Expanded star exprs are not rewritten. + RewritesOk("select * from (" + stmt_ + ") v", 23, 11); + // Test union, 11 + 11 + 1 rewritable Expr trees. + RewritesOk(String.format("%s union all (%s) order by cnt", stmt_, stmt_), 47, 23); + // Test union inside an inline view. + RewritesOk(String.format("select * from (%s union all (%s) order by cnt limit 10) v", + stmt_, stmt_), 47, 23); + // Constant select. + RewritesOk("select 1, 2, 3, 4", 4, 4); + // Values stmt. + RewritesOk("values(1, '2', 3, 4.1), (1, '2', 3, 4.1)", 8, 8); + // Test WHERE-clause subqueries. + RewritesOk("select id, int_col from functional.alltypes a " + + "where exists (select 1 from functional.alltypes " + + "where string_col = 'test' having count(*) < 10)", 9, 5); + RewritesOk("select id, int_col from functional.alltypes a " + + "where a.id in (select count(*) from functional.alltypes " + + "where string_col = 'test' having count(*) < 10)", 10, 6); + } + + @Test + public void TestDdlStmts() throws AnalysisException { + RewritesOk("create table ctas_test as " + stmt_, 23, 11); + // Create/alter view stmts are not rewritten to preserve the original SQL. + CheckNoRewrite("create view view_test as " + stmt_); + CheckNoRewrite("alter view functional.alltypes_view as " + stmt_); + } + + @Test + public void TestDmlStmts() throws AnalysisException { + // Insert. + RewritesOk("insert into functional.alltypes (id, int_col, float_col, bigint_col) " + + "partition(year=2009,month=10) " + stmt_, 23, 11); + + if (RuntimeEnv.INSTANCE.isKuduSupported()) { + // Update. + RewritesOk("update t2 set name = 'test' from " + + "functional.alltypes t1 join functional_kudu.dimtbl t2 on (t1.id = t2.id) " + + "where t2.id < 10", 10, 5); + RewritesOk("update functional_kudu.dimtbl set name = 'test', zip = 4711 " + + "where exists (" + stmt_ + ")", 28, 16); + // Delete. + RewritesOk("delete a from " + + "functional_kudu.testtbl a join functional.testtbl b on a.zip = b.zip", 4, 2); + RewritesOk("delete functional_kudu.testtbl where exists (" + stmt_ + ")", 24, 12); + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java index 0a9ad66..3244b20 100644 --- a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java +++ b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java @@ -25,9 +25,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import org.junit.After; -import org.junit.Assert; - import org.apache.impala.analysis.AnalysisContext; import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.ColumnDef; @@ -57,6 +54,9 @@ import org.apache.impala.testutil.TestUtils; import org.apache.impala.thrift.TFunctionBinaryType; import org.apache.impala.thrift.TQueryCtx; import org.apache.impala.thrift.TQueryOptions; +import org.junit.After; +import org.junit.Assert; + import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -183,7 +183,9 @@ public class FrontendTestBase { CreateViewStmt createViewStmt = (CreateViewStmt) AnalyzesOk(createViewSql); Db db = catalog_.getDb(createViewStmt.getDb()); Preconditions.checkNotNull(db, "Test views must be created in an existing db."); - QueryStmt viewStmt = (QueryStmt) AnalyzesOk(createViewStmt.getInlineViewDef()); + // Do not analyze the stmt to avoid applying rewrites that would alter the view + // definition. We want to model real views as closely as possible. + QueryStmt viewStmt = (QueryStmt) ParsesOk(createViewStmt.getInlineViewDef()); View dummyView = View.createTestView(db, createViewStmt.getTbl(), viewStmt); db.addTable(dummyView); testTables_.add(dummyView); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test index 8144524..c4cbd6f 100644 --- a/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test +++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test @@ -55,7 +55,7 @@ PLAN-ROOT SINK | 00:SCAN HDFS [tpcds.date_dim dt] partitions=1/1 files=1 size=9.84MB - predicates: dt.d_moy = 12, (dt.d_date_sk BETWEEN 2451149 AND 2451179 OR dt.d_date_sk BETWEEN 2451514 AND 2451544 OR dt.d_date_sk BETWEEN 2451880 AND 2451910 OR dt.d_date_sk BETWEEN 2452245 AND 2452275 OR dt.d_date_sk BETWEEN 2452610 AND 2452640) + predicates: dt.d_moy = 12, (dt.d_date_sk >= 2451149 AND dt.d_date_sk <= 2451179 OR dt.d_date_sk >= 2451514 AND dt.d_date_sk <= 2451544 OR dt.d_date_sk >= 2451880 AND dt.d_date_sk <= 2451910 OR dt.d_date_sk >= 2452245 AND dt.d_date_sk <= 2452275 OR dt.d_date_sk >= 2452610 AND dt.d_date_sk <= 2452640) runtime filters: RF000 -> dt.d_date_sk ---- DISTRIBUTEDPLAN PLAN-ROOT SINK @@ -99,7 +99,7 @@ PLAN-ROOT SINK | 00:SCAN HDFS [tpcds.date_dim dt] partitions=1/1 files=1 size=9.84MB - predicates: dt.d_moy = 12, (dt.d_date_sk BETWEEN 2451149 AND 2451179 OR dt.d_date_sk BETWEEN 2451514 AND 2451544 OR dt.d_date_sk BETWEEN 2451880 AND 2451910 OR dt.d_date_sk BETWEEN 2452245 AND 2452275 OR dt.d_date_sk BETWEEN 2452610 AND 2452640) + predicates: dt.d_moy = 12, (dt.d_date_sk >= 2451149 AND dt.d_date_sk <= 2451179 OR dt.d_date_sk >= 2451514 AND dt.d_date_sk <= 2451544 OR dt.d_date_sk >= 2451880 AND dt.d_date_sk <= 2451910 OR dt.d_date_sk >= 2452245 AND dt.d_date_sk <= 2452275 OR dt.d_date_sk >= 2452610 AND dt.d_date_sk <= 2452640) runtime filters: RF000 -> dt.d_date_sk ---- PARALLELPLANS PLAN-ROOT SINK @@ -151,7 +151,7 @@ PLAN-ROOT SINK | 00:SCAN HDFS [tpcds.date_dim dt] partitions=1/1 files=1 size=9.84MB - predicates: dt.d_moy = 12, (dt.d_date_sk BETWEEN 2451149 AND 2451179 OR dt.d_date_sk BETWEEN 2451514 AND 2451544 OR dt.d_date_sk BETWEEN 2451880 AND 2451910 OR dt.d_date_sk BETWEEN 2452245 AND 2452275 OR dt.d_date_sk BETWEEN 2452610 AND 2452640) + predicates: dt.d_moy = 12, (dt.d_date_sk >= 2451149 AND dt.d_date_sk <= 2451179 OR dt.d_date_sk >= 2451514 AND dt.d_date_sk <= 2451544 OR dt.d_date_sk >= 2451880 AND dt.d_date_sk <= 2451910 OR dt.d_date_sk >= 2452245 AND dt.d_date_sk <= 2452275 OR dt.d_date_sk >= 2452610 AND dt.d_date_sk <= 2452640) runtime filters: RF000 -> dt.d_date_sk ==== # TPCDS-Q7 @@ -1209,7 +1209,7 @@ PLAN-ROOT SINK | | | |--01:SCAN HDFS [tpcds.date_dim] | | partitions=1/1 files=1 size=9.84MB -| | predicates: date_dim.d_year IN (1998, 1998 + 1, 1998 + 2), (date_dim.d_dom BETWEEN 1 AND 3 OR date_dim.d_dom BETWEEN 25 AND 28) +| | predicates: date_dim.d_year IN (1998, 1998 + 1, 1998 + 2), (date_dim.d_dom >= 1 AND date_dim.d_dom <= 3 OR date_dim.d_dom >= 25 AND date_dim.d_dom <= 28) | | | 04:HASH JOIN [INNER JOIN] | | hash predicates: store_sales.ss_hdemo_sk = household_demographics.hd_demo_sk @@ -1271,7 +1271,7 @@ PLAN-ROOT SINK | | | | | 01:SCAN HDFS [tpcds.date_dim] | | partitions=1/1 files=1 size=9.84MB -| | predicates: date_dim.d_year IN (1998, 1998 + 1, 1998 + 2), (date_dim.d_dom BETWEEN 1 AND 3 OR date_dim.d_dom BETWEEN 25 AND 28) +| | predicates: date_dim.d_year IN (1998, 1998 + 1, 1998 + 2), (date_dim.d_dom >= 1 AND date_dim.d_dom <= 3 OR date_dim.d_dom >= 25 AND date_dim.d_dom <= 28) | | | 04:HASH JOIN [INNER JOIN, BROADCAST] | | hash predicates: store_sales.ss_hdemo_sk = household_demographics.hd_demo_sk @@ -1347,7 +1347,7 @@ PLAN-ROOT SINK | | | | | 01:SCAN HDFS [tpcds.date_dim] | | partitions=1/1 files=1 size=9.84MB -| | predicates: date_dim.d_year IN (1998, 1998 + 1, 1998 + 2), (date_dim.d_dom BETWEEN 1 AND 3 OR date_dim.d_dom BETWEEN 25 AND 28) +| | predicates: date_dim.d_year IN (1998, 1998 + 1, 1998 + 2), (date_dim.d_dom >= 1 AND date_dim.d_dom <= 3 OR date_dim.d_dom >= 25 AND date_dim.d_dom <= 28) | | | 04:HASH JOIN [INNER JOIN, BROADCAST] | | hash predicates: store_sales.ss_hdemo_sk = household_demographics.hd_demo_sk http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test index d883b74..9b17ff1 100644 --- a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test +++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test @@ -3415,7 +3415,7 @@ PLAN-ROOT SINK | 02:HASH JOIN [INNER JOIN] | hash predicates: l_partkey = p_partkey -| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size BETWEEN 1 AND 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size BETWEEN 1 AND 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size BETWEEN 1 AND 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) +| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) | runtime filters: RF000 <- p_partkey | |--01:SCAN HDFS [tpch.part] @@ -3437,7 +3437,7 @@ PLAN-ROOT SINK | 02:HASH JOIN [INNER JOIN, BROADCAST] | hash predicates: l_partkey = p_partkey -| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size BETWEEN 1 AND 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size BETWEEN 1 AND 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size BETWEEN 1 AND 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) +| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) | runtime filters: RF000 <- p_partkey | |--04:EXCHANGE [BROADCAST] @@ -3461,7 +3461,7 @@ PLAN-ROOT SINK | 02:HASH JOIN [INNER JOIN, BROADCAST] | hash predicates: l_partkey = p_partkey -| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size BETWEEN 1 AND 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size BETWEEN 1 AND 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size BETWEEN 1 AND 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) +| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) | runtime filters: RF000 <- p_partkey | |--JOIN BUILD http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1d8cdb02/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test index 0b7f35d..a5e6e07 100644 --- a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test +++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test @@ -1074,7 +1074,7 @@ PLAN-ROOT SINK | 02:HASH JOIN [INNER JOIN] | hash predicates: l_partkey = p_partkey -| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size BETWEEN 1 AND 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size BETWEEN 1 AND 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size BETWEEN 1 AND 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) +| other predicates: ((p_brand = 'Brand#12' AND p_container IN ('SM CASE', 'SM BOX', 'SM PACK', 'SM PKG') AND l_quantity >= 1 AND l_quantity <= 11 AND p_size >= 1 AND p_size <= 5 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#23' AND p_container IN ('MED BAG', 'MED BOX', 'MED PKG', 'MED PACK') AND l_quantity >= 10 AND l_quantity <= 20 AND p_size >= 1 AND p_size <= 10 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON') OR (p_brand = 'Brand#34' AND p_container IN ('LG CASE', 'LG BOX', 'LG PACK', 'LG PKG') AND l_quantity >= 20 AND l_quantity <= 30 AND p_size >= 1 AND p_size <= 15 AND l_shipmode IN ('AIR', 'AIR REG') AND l_shipinstruct = 'DELIVER IN PERSON')) | |--01:SCAN KUDU [tpch_kudu.part] |
