This is an automated email from the ASF dual-hosted git repository. joemcdonnell pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 026ef05da84323512576aa7934cbce2440abd118 Author: Steve Carlin <[email protected]> AuthorDate: Fri Jan 3 14:16:53 2025 -0800 IMPALA-13653: Create hooks for Calcite planner in Frontend This commit creates hooks into the Impala planner that will call into the Calcite planner. There is one factory hook and 3 compilation hooks introduced to allow this to happen: - CompilerFactory : This factory hook has methods to create the implementations for the three interfaces described below. The idea is to introduce a query option that allows the Calcite objects to be called instead of the Impala planner. In this iteration, the only implementation called is the Impala specific implementations. - ParsedStatement: The implementation of this interface will parse the SQL query into an AST in its constructor. The implementation will be a wrapper for the AST which can be accessed through the "getTopLevelNode" method. For the current planner, the "StatementBase" object will be accessible via this method. - AnalysisDriver: The implementation of this interface will analyze the parsed AST via the "analyze" method. To make the code review as easy as possible, the AnalyzeDriverImpl implementation has been placed inside the AnalysisContext object. - SingleNodePlannerIntf: The implementation of this interface will return a PlanNode that can run on a single node via the "createSingleNodePlan" method. Various other methods in this interface allow access to data structures that are needed for the final plan. These objects are related to the output Expr objects. Change-Id: I978aa48b160984ee5d36244cd915940f838d141d Reviewed-on: http://gerrit.cloudera.org:8080/22306 Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Aman Sinha <[email protected]> --- .../apache/impala/analysis/AnalysisContext.java | 63 +++++++++++------ .../apache/impala/analysis/ParsedStatement.java | 44 ++++++++++++ .../impala/analysis/ParsedStatementImpl.java | 79 ++++++++++++++++++++++ .../apache/impala/analysis/StmtMetadataLoader.java | 6 +- .../java/org/apache/impala/planner/Planner.java | 26 ++++--- .../apache/impala/planner/SingleNodePlanner.java | 36 +++++++++- .../impala/planner/SingleNodePlannerIntf.java | 49 ++++++++++++++ .../org/apache/impala/service/CompilerFactory.java | 58 ++++++++++++++++ .../apache/impala/service/CompilerFactoryImpl.java | 63 +++++++++++++++++ .../java/org/apache/impala/service/Frontend.java | 75 ++++++++++---------- .../impala/analysis/ExprRewriteRulesTest.java | 7 +- .../apache/impala/analysis/ExprRewriterTest.java | 14 ++-- .../org/apache/impala/analysis/ParserTest.java | 30 ++++---- .../impala/analysis/StmtMetadataLoaderTest.java | 12 ++-- .../org/apache/impala/common/FrontendFixture.java | 24 ++++--- .../org/apache/impala/common/FrontendTestBase.java | 22 ++++-- .../org/apache/impala/common/QueryFixture.java | 23 ++++--- 17 files changed, 507 insertions(+), 124 deletions(-) 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 6d3629008..ead7d4a3e 100644 --- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java +++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java @@ -36,6 +36,7 @@ import org.apache.impala.common.AnalysisException; import org.apache.impala.common.ImpalaException; import org.apache.impala.common.RuntimeEnv; import org.apache.impala.rewrite.ExprRewriter; +import org.apache.impala.service.CompilerFactory; import org.apache.impala.thrift.TAccessEvent; import org.apache.impala.thrift.TClientRequest; import org.apache.impala.thrift.TLineageGraph; @@ -87,17 +88,26 @@ public class AnalysisContext { } static public class AnalysisResult { + // The wrapper for the parsed AST for a planner (e.g. original, Calcite). + private final ParsedStatement parsedStmt_; + // AST for original planner. This is retrievable via the parsedStmt_ variable + // but in its own variable for convenience. For the Calcite planner, this will be + // null. private final StatementBase stmt_; private final Analyzer analyzer_; private final ImpalaException exception_; private boolean userHasProfileAccess_ = true; - public AnalysisResult(StatementBase stmt, Analyzer analyzer) { - this(stmt, analyzer, null); + public AnalysisResult(ParsedStatement parsedStmt, Analyzer analyzer) { + this(parsedStmt, analyzer, null); } - public AnalysisResult(StatementBase stmt, Analyzer analyzer, ImpalaException e) { - stmt_ = stmt; + public AnalysisResult(ParsedStatement parsedStmt, Analyzer analyzer, + ImpalaException e) { + parsedStmt_ = parsedStmt; + stmt_ = parsedStmt_.getTopLevelNode() instanceof StatementBase + ? (StatementBase) parsedStmt_.getTopLevelNode() + : null; analyzer_ = analyzer; exception_ = e; } @@ -105,7 +115,7 @@ public class AnalysisContext { public boolean isAlterTableStmt() { return stmt_ instanceof AlterTableStmt; } public boolean isAlterViewStmt() { return stmt_ instanceof AlterViewStmt; } public boolean isComputeStatsStmt() { return stmt_ instanceof ComputeStatsStmt; } - public boolean isQueryStmt() { return stmt_ instanceof QueryStmt; } + public boolean isQueryStmt() { return parsedStmt_.isQueryStmt(); } public boolean isSetOperationStmt() { return stmt_ instanceof SetOperationStmt; } public boolean isInsertStmt() { return stmt_ instanceof InsertStmt; } public boolean isMergeStmt() { return stmt_ instanceof MergeStmt; } @@ -155,7 +165,7 @@ public class AnalysisContext { public boolean isDescribeDbStmt() { return stmt_ instanceof DescribeDbStmt; } public boolean isDescribeTableStmt() { return stmt_ instanceof DescribeTableStmt; } public boolean isResetMetadataStmt() { return stmt_ instanceof ResetMetadataStmt; } - public boolean isExplainStmt() { return stmt_.isExplain(); } + public boolean isExplainStmt() { return parsedStmt_.isExplain(); } public boolean isShowRolesStmt() { return stmt_ instanceof ShowRolesStmt; } public boolean isShowGrantPrincipalStmt() { return stmt_ instanceof ShowGrantPrincipalStmt; @@ -440,6 +450,7 @@ public class AnalysisContext { return (KillQueryStmt) stmt_; } + public ParsedStatement getParsedStmt() { return parsedStmt_; } public StatementBase getStmt() { return stmt_; } public Analyzer getAnalyzer() { return analyzer_; } public ImpalaException getException() { return exception_; } @@ -452,11 +463,11 @@ public class AnalysisContext { } - public AnalysisResult analyzeAndAuthorize(StatementBase stmt, - StmtTableCache stmtTableCache, AuthorizationChecker authzChecker) - throws ImpalaException { - return analyzeAndAuthorize( - stmt, stmtTableCache, authzChecker, false /*disableAuthorization*/); + public AnalysisResult analyzeAndAuthorize(CompilerFactory compilerFactory, + ParsedStatement parsedStmt, StmtTableCache stmtTableCache, + AuthorizationChecker authzChecker) throws ImpalaException { + return analyzeAndAuthorize(compilerFactory, parsedStmt, + stmtTableCache, authzChecker, false /*disableAuthorization*/); } /** @@ -465,8 +476,9 @@ public class AnalysisContext { * AuthorizationExceptions take precedence over AnalysisExceptions so as not to * reveal the existence/absence of objects the user is not authorized to see. */ - public AnalysisResult analyzeAndAuthorize(StatementBase stmt, - StmtTableCache stmtTableCache, AuthorizationChecker authzChecker, + public AnalysisResult analyzeAndAuthorize(CompilerFactory compilerFactory, + ParsedStatement parsedStmt, StmtTableCache stmtTableCache, + AuthorizationChecker authzChecker, boolean disableAuthorization) throws ImpalaException { // TODO: Clean up the creation/setting of the analysis result. catalog_ = stmtTableCache.catalog; @@ -478,14 +490,16 @@ public class AnalysisContext { clientRequest.getRedacted_stmt() : clientRequest.getStmt(), queryCtx_.getSession(), Optional.of(timeline_)); Preconditions.checkState(authzCtx != null); - AnalysisDriverImpl analysisDriver = - new AnalysisDriverImpl(this, stmt, stmtTableCache, authzCtx); + AnalysisDriver analysisDriver = compilerFactory.createAnalysisDriver(this, + parsedStmt, stmtTableCache, authzCtx); analysisResult_ = analysisDriver.analyze(); authzChecker.postAnalyze(authzCtx); ImpalaException analysisException = analysisResult_.getException(); // A statement that returns at most one row does not need to spool query results. + // IMPALA-13902: returnsAtMostOneRow should be in planner interface so it is + // accessible by the Calcite planner. if (analysisException == null && analysisResult_.getStmt() instanceof SelectStmt && ((SelectStmt)analysisResult_.getStmt()).returnsAtMostOneRow()) { clientRequest.query_options.setSpool_query_results(false); @@ -529,18 +543,23 @@ public class AnalysisContext { * stage. */ public static class AnalysisDriverImpl implements AnalysisDriver { + // the stmt variables and Analyzer are not final variables because of the + // rewrite code. This should perhaps be refactored in the future. + private ParsedStatement parsedStmt_; private StatementBase stmt_; private Analyzer analyzer_; private final AnalysisContext ctx_; private final StmtTableCache stmtTableCache_; private final AuthorizationContext authzCtx_; - public AnalysisDriverImpl(AnalysisContext ctx, StatementBase stmt, + public AnalysisDriverImpl(AnalysisContext ctx, ParsedStatement parsedStmt, StmtTableCache stmtTableCache, AuthorizationContext authzCtx) { - Preconditions.checkNotNull(stmt); + Preconditions.checkNotNull(parsedStmt); + parsedStmt_ = parsedStmt; ctx_ = ctx; - stmt_ = stmt; + Preconditions.checkNotNull(parsedStmt.getTopLevelNode()); + stmt_ = (StatementBase) parsedStmt.getTopLevelNode(); stmtTableCache_ = stmtTableCache; authzCtx_ = authzCtx; analyzer_ = createAnalyzer(ctx_, stmtTableCache, authzCtx); @@ -615,7 +634,7 @@ public class AnalysisContext { shouldReAnalyze = true; } if (!shouldReAnalyze) { - return new AnalysisResult(stmt_, analyzer_); + return new AnalysisResult(parsedStmt_, analyzer_); } // For SetOperationStmt we must replace the query statement with the rewritten @@ -626,6 +645,7 @@ public class AnalysisContext { if (((SetOperationStmt) stmt_).hasRewrittenStmt()) { boolean isExplain = stmt_.isExplain(); stmt_ = ((SetOperationStmt) stmt_).getRewrittenStmt(); + parsedStmt_ = new ParsedStatementImpl(stmt_); if (isExplain) stmt_.setIsExplain(); } } @@ -634,9 +654,9 @@ public class AnalysisContext { reAnalyze(stmtTableCache_, authzCtx_, origResultTypes, origColLabels, /*collectPrivileges*/ false); Preconditions.checkState(!requiresSubqueryRewrite()); - return new AnalysisResult(stmt_, analyzer_); + return new AnalysisResult(parsedStmt_, analyzer_); } catch (ImpalaException e) { - return new AnalysisResult(stmt_, analyzer_, e); + return new AnalysisResult(parsedStmt_, analyzer_, e); } } @@ -725,6 +745,7 @@ public class AnalysisContext { public boolean isCreateTableAsSelectStmt() { return stmt_ instanceof CreateTableAsSelectStmt; } + private boolean isZippingUnnestInSelectList(StatementBase stmt) { if (!(stmt instanceof SelectStmt)) return false; if (!stmt.analyzer_.getTableRefsFromUnnestExpr().isEmpty()) return true; diff --git a/fe/src/main/java/org/apache/impala/analysis/ParsedStatement.java b/fe/src/main/java/org/apache/impala/analysis/ParsedStatement.java new file mode 100644 index 000000000..6d89d199b --- /dev/null +++ b/fe/src/main/java/org/apache/impala/analysis/ParsedStatement.java @@ -0,0 +1,44 @@ +// 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.Set; + +/** + * ParsedStatement interface that holds the parsed query statement. + * + * This is currently implemented by the original Impala parser and the + * Calcite parser. + */ +public interface ParsedStatement { + + // Retrieve all the tables/views referenced in the parsed query. + public Set<TableName> getTablesInQuery(StmtMetadataLoader loader); + + // return the wrapped statement object. + public Object getTopLevelNode(); + + // true if this is an explain statement + public boolean isExplain(); + + // true if this is a query (select) statement + public boolean isQueryStmt(); + + // returns the sql string (could be rewritten) + public String toSql(); +} diff --git a/fe/src/main/java/org/apache/impala/analysis/ParsedStatementImpl.java b/fe/src/main/java/org/apache/impala/analysis/ParsedStatementImpl.java new file mode 100644 index 000000000..faeb1d40a --- /dev/null +++ b/fe/src/main/java/org/apache/impala/analysis/ParsedStatementImpl.java @@ -0,0 +1,79 @@ +// 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.Set; + +import org.apache.impala.common.ImpalaException; +import org.apache.impala.thrift.TQueryCtx; +import org.apache.impala.thrift.TQueryOptions; + +/** + * Implementation of the ParsedStatement for the original Impala planner. On + * construction, the sql statement will be parsed. + */ +public class ParsedStatementImpl implements ParsedStatement { + + // the wrapped sql statement object. Note, this is not final because Impala + // allows the statement to be rewritten at analysis time (which is kinda + // hacky) + private final StatementBase stmt_ ; + + public ParsedStatementImpl(TQueryCtx queryCtx) throws ImpalaException { + stmt_ = Parser.parse(queryCtx.client_request.stmt, + queryCtx.client_request.query_options); + } + + public ParsedStatementImpl(String stmt) throws ImpalaException { + stmt_ = Parser.parse(stmt); + } + + public ParsedStatementImpl(String stmt, TQueryOptions queryOpt) throws ImpalaException { + stmt_ = Parser.parse(stmt, queryOpt); + } + + public ParsedStatementImpl(StatementBase stmt) { + stmt_ = stmt; + } + + @Override + public Set<TableName> getTablesInQuery(StmtMetadataLoader loader) { + return loader.collectTableCandidates(stmt_); + } + + // Retrieves the wrapped sql object + @Override + public Object getTopLevelNode() { + return stmt_; + } + + @Override + public boolean isExplain() { + return stmt_.isExplain(); + } + + @Override + public boolean isQueryStmt() { + return stmt_ instanceof QueryStmt; + } + + @Override + public String toSql() { + return stmt_.toSql(); + } +} diff --git a/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java b/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java index ef0099ddd..fcac20df4 100644 --- a/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java +++ b/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java @@ -140,8 +140,8 @@ public class StmtMetadataLoader { * Marks the start and end of metadata loading in 'timeline_' if it is non-NULL. * Must only be called once for a single statement. */ - public StmtTableCache loadTables(StatementBase stmt) throws InternalException { - Set<TableName> requiredTables = collectTableCandidates(stmt); + public StmtTableCache loadTables(ParsedStatement stmt) throws InternalException { + Set<TableName> requiredTables = stmt.getTablesInQuery(this); return loadTables(requiredTables); } @@ -366,7 +366,7 @@ public class StmtMetadataLoader { * and generating all possible table-path resolutions considered during analysis. * Uses 'sessionDb_' to construct the candidate tables with Path.getCandidateTables(). */ - private Set<TableName> collectTableCandidates(StatementBase stmt) { + public Set<TableName> collectTableCandidates(StatementBase stmt) { Preconditions.checkNotNull(stmt); List<TableRef> tblRefs = new ArrayList<>(); // The information about whether table masking is supported is not available to diff --git a/fe/src/main/java/org/apache/impala/planner/Planner.java b/fe/src/main/java/org/apache/impala/planner/Planner.java index 131a268e6..259f8d613 100644 --- a/fe/src/main/java/org/apache/impala/planner/Planner.java +++ b/fe/src/main/java/org/apache/impala/planner/Planner.java @@ -39,6 +39,7 @@ import org.apache.impala.analysis.InsertStmt; import org.apache.impala.analysis.JoinOperator; import org.apache.impala.analysis.MergeCase; import org.apache.impala.analysis.MergeStmt; +import org.apache.impala.analysis.ParsedStatement; import org.apache.impala.analysis.QueryStmt; import org.apache.impala.analysis.SortInfo; import org.apache.impala.analysis.TupleId; @@ -51,12 +52,14 @@ import org.apache.impala.common.Pair; import org.apache.impala.common.PrintUtils; import org.apache.impala.common.RuntimeEnv; import org.apache.impala.service.BackendConfig; +import org.apache.impala.service.CompilerFactory; import org.apache.impala.thrift.QueryConstants; import org.apache.impala.thrift.TExplainLevel; import org.apache.impala.thrift.TMinmaxFilteringLevel; import org.apache.impala.thrift.TQueryCtx; import org.apache.impala.thrift.TQueryExecRequest; import org.apache.impala.thrift.TQueryOptions; +import org.apache.impala.thrift.TResultSetMetadata; import org.apache.impala.thrift.TRuntimeFilterMode; import org.apache.impala.thrift.TTableName; import org.apache.impala.util.EventSequence; @@ -101,9 +104,12 @@ public class Planner { private final PlannerContext ctx_; - public Planner(AnalysisResult analysisResult, TQueryCtx queryCtx, - EventSequence timeline) { + private final SingleNodePlannerIntf singleNodePlannerIntf_; + + public Planner(CompilerFactory compilerFactory, AnalysisResult analysisResult, + TQueryCtx queryCtx, EventSequence timeline) throws ImpalaException { ctx_ = new PlannerContext(analysisResult, queryCtx, timeline); + singleNodePlannerIntf_ = compilerFactory.createSingleNodePlanner(ctx_); } public TQueryCtx getQueryCtx() { return ctx_.getQueryCtx(); } @@ -127,9 +133,8 @@ public class Planner { * that typically means there is a bug in analysis, or a broken/missing smap. */ private List<PlanFragment> createPlanFragments() throws ImpalaException { - SingleNodePlanner singleNodePlanner = new SingleNodePlanner(ctx_); DistributedPlanner distributedPlanner = new DistributedPlanner(ctx_); - PlanNode singleNodePlan = singleNodePlanner.createSingleNodePlan(); + PlanNode singleNodePlan = singleNodePlannerIntf_.createSingleNodePlan(); ctx_.getTimeline().markEvent("Single node plan created"); List<PlanFragment> fragments = null; @@ -198,11 +203,7 @@ public class Planner { // Set up update sink for root fragment rootFragment.setSink(stmt.createDataSink()); } else if (ctx_.isQuery()) { - QueryStmt queryStmt = ctx_.getQueryStmt(); - queryStmt.substituteResultExprs(rootNodeSmap, ctx_.getRootAnalyzer()); - List<Expr> resultExprs = queryStmt.getResultExprs(); - rootFragment.setSink( - ctx_.getAnalysisResult().getQueryStmt().createDataSink(resultExprs)); + rootFragment.setSink(singleNodePlannerIntf_.createDataSink(rootNodeSmap)); } // The check for disabling codegen uses estimates of rows per node so must be done @@ -270,7 +271,8 @@ public class Planner { graph.addTargetColumnLabels(targetTable); } } else { - graph.addTargetColumnLabels(ctx_.getQueryStmt().getColLabels().stream() + List<String> colLabels = singleNodePlannerIntf_.getColLabels(); + graph.addTargetColumnLabels(colLabels.stream() .map(col -> new ColumnLabel(col)) .collect(Collectors.toList())); } @@ -1038,6 +1040,10 @@ public class Planner { inputFragment.setPlanRoot(node); } + public TResultSetMetadata getTResultSetMetadata(ParsedStatement parsedStmt) { + return singleNodePlannerIntf_.getTResultSetMetadata(parsedStmt); + } + private PlanNode addMergeNode(PlanNode planNode, Analyzer analyzer) throws ImpalaException { MergeStmt merge = ctx_.getAnalysisResult().getMergeStmt(); diff --git a/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java b/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java index 0a5ad5986..921cdd8cf 100644 --- a/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java +++ b/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java @@ -44,6 +44,7 @@ import org.apache.impala.analysis.JoinOperator; import org.apache.impala.analysis.MultiAggregateInfo; import org.apache.impala.analysis.MultiAggregateInfo.AggPhase; import org.apache.impala.analysis.NullLiteral; +import org.apache.impala.analysis.ParsedStatement; import org.apache.impala.analysis.Path; import org.apache.impala.analysis.Path.PathType; import org.apache.impala.analysis.QueryStmt; @@ -80,7 +81,9 @@ import org.apache.impala.common.NotImplementedException; import org.apache.impala.common.Pair; import org.apache.impala.planner.AnalyticEvalNode.LimitPushdownInfo; import org.apache.impala.planner.JoinNode.DistributionMode; +import org.apache.impala.thrift.TColumn; import org.apache.impala.thrift.TQueryOptions; +import org.apache.impala.thrift.TResultSetMetadata; import org.apache.impala.util.AcidUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -97,7 +100,7 @@ import com.google.common.collect.Sets; * such as local aggregations that are important for distributed execution. * The single-node plan needs to be wrapped in a plan fragment for it to be executable. */ -public class SingleNodePlanner { +public class SingleNodePlanner implements SingleNodePlannerIntf { // Controls whether a distinct aggregation should be inserted before a join input. // If the size of the distinct values after aggregation is less than or equal to // the original input size multiplied by this threshold, the distinct agg should be @@ -2359,4 +2362,35 @@ public class SingleNodePlanner { } return result; } + + @Override + public DataSink createDataSink(ExprSubstitutionMap rootNodeSmap) { + QueryStmt queryStmt = ctx_.getQueryStmt(); + queryStmt.substituteResultExprs(rootNodeSmap, ctx_.getRootAnalyzer()); + List<Expr> resultExprs = queryStmt.getResultExprs(); + return ctx_.getAnalysisResult().getQueryStmt().createDataSink(resultExprs); + } + + @Override + public List<String> getColLabels() { + return ctx_.getQueryStmt().getColLabels(); + } + + /** + * fetch the metadata for the result set + */ + @Override + public TResultSetMetadata getTResultSetMetadata(ParsedStatement parsedStmt) { + LOG.trace("create result set metadata"); + TResultSetMetadata metadata = new TResultSetMetadata(); + QueryStmt queryStmt = (QueryStmt) parsedStmt.getTopLevelNode(); + int colCnt = queryStmt.getColLabels().size(); + for (int i = 0; i < colCnt; ++i) { + TColumn colDesc = new TColumn(); + colDesc.columnName = queryStmt.getColLabels().get(i); + colDesc.columnType = queryStmt.getResultExprs().get(i).getType().toThrift(); + metadata.addToColumns(colDesc); + } + return metadata; + } } diff --git a/fe/src/main/java/org/apache/impala/planner/SingleNodePlannerIntf.java b/fe/src/main/java/org/apache/impala/planner/SingleNodePlannerIntf.java new file mode 100644 index 000000000..07788c8a1 --- /dev/null +++ b/fe/src/main/java/org/apache/impala/planner/SingleNodePlannerIntf.java @@ -0,0 +1,49 @@ +// 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.planner; + +import org.apache.impala.analysis.ExprSubstitutionMap; +import org.apache.impala.analysis.ParsedStatement; +import org.apache.impala.common.ImpalaException; +import org.apache.impala.thrift.TResultSetMetadata; + +import java.util.List; + +/** + * SingleNodePlanner interface that creates the physical non-distributed + * plan. + * + * This is currently implemented by the original Impala planner and the + * Calcite planner. + */ +public interface SingleNodePlannerIntf { + + // Main method: Create the PlanNode which is the step after analysis + // in the compilation process. + public PlanNode createSingleNodePlan() throws ImpalaException; + + // Create the top level DataSink. + public DataSink createDataSink(ExprSubstitutionMap rootNodeSmap); + + // Create the column labels for the output expressions + public List<String> getColLabels(); + + // Create the TResultSetMetadata which contains the output expressions + public TResultSetMetadata getTResultSetMetadata(ParsedStatement stmt); + +} diff --git a/fe/src/main/java/org/apache/impala/service/CompilerFactory.java b/fe/src/main/java/org/apache/impala/service/CompilerFactory.java new file mode 100644 index 000000000..adf414f6f --- /dev/null +++ b/fe/src/main/java/org/apache/impala/service/CompilerFactory.java @@ -0,0 +1,58 @@ +// 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.service; + +import org.apache.impala.analysis.AnalysisContext; +import org.apache.impala.analysis.AnalysisDriver; +import org.apache.impala.analysis.ParsedStatement; +import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache; +import org.apache.impala.authorization.AuthorizationContext; +import org.apache.impala.common.AnalysisException; +import org.apache.impala.common.ImpalaException; +import org.apache.impala.planner.SingleNodePlannerIntf; +import org.apache.impala.planner.PlannerContext; +import org.apache.impala.thrift.TQueryCtx; +import org.apache.impala.thrift.TQueryOptions; + +/** + * CompilerFactory is the factory interface which provides methods to + * allow various alternatives of compilation. + */ +public interface CompilerFactory { + + /** + * Create a ParsedStatement class which does the parsing of the query. + */ + public ParsedStatement createParsedStatement(TQueryCtx queryCtx) throws ImpalaException; + + /** + * Create an analysis driver handling the implementation specific parts of analysis. + */ + public AnalysisDriver createAnalysisDriver(AnalysisContext ctx, + ParsedStatement parsedStmt, StmtTableCache stmtTableCache, + AuthorizationContext authzCtx) throws AnalysisException; + + /** + * Create a single node planner handling the implementation specific parts of the + * single node planning. + */ + public SingleNodePlannerIntf createSingleNodePlanner(PlannerContext ctx) + throws ImpalaException; + + public String getPlannerString(); +} diff --git a/fe/src/main/java/org/apache/impala/service/CompilerFactoryImpl.java b/fe/src/main/java/org/apache/impala/service/CompilerFactoryImpl.java new file mode 100644 index 000000000..77b6090fb --- /dev/null +++ b/fe/src/main/java/org/apache/impala/service/CompilerFactoryImpl.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.service; + +import org.apache.impala.analysis.AnalysisContext; +import org.apache.impala.analysis.AnalysisContext.AnalysisDriverImpl; +import org.apache.impala.analysis.AnalysisDriver; +import org.apache.impala.analysis.ParsedStatement; +import org.apache.impala.analysis.ParsedStatementImpl; +import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache; +import org.apache.impala.authorization.AuthorizationContext; +import org.apache.impala.common.AnalysisException; +import org.apache.impala.common.ImpalaException; +import org.apache.impala.planner.SingleNodePlanner; +import org.apache.impala.planner.SingleNodePlannerIntf; +import org.apache.impala.planner.PlannerContext; +import org.apache.impala.thrift.TQueryCtx; +import org.apache.impala.thrift.TQueryOptions; + + +/** + * Factory implementation of the original Impala planner which creates + * Compiler implementation classes. + */ +public class CompilerFactoryImpl implements CompilerFactory { + + public static final String PLANNER = "OriginalPlanner"; + + public ParsedStatement createParsedStatement(TQueryCtx queryCtx) + throws ImpalaException { + return new ParsedStatementImpl(queryCtx); + } + + public AnalysisDriver createAnalysisDriver(AnalysisContext ctx, + ParsedStatement parsedStmt, StmtTableCache stmtTableCache, + AuthorizationContext authzCtx) throws AnalysisException { + return new AnalysisDriverImpl(ctx, parsedStmt, stmtTableCache, authzCtx); + } + + public SingleNodePlannerIntf createSingleNodePlanner(PlannerContext ctx) + throws ImpalaException { + return new SingleNodePlanner(ctx); + } + + public String getPlannerString() { + return PLANNER; + } +} diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java index 9abfdb1c1..40c7df2a7 100644 --- a/fe/src/main/java/org/apache/impala/service/Frontend.java +++ b/fe/src/main/java/org/apache/impala/service/Frontend.java @@ -89,6 +89,7 @@ import org.apache.impala.analysis.GrantRevokePrivStmt; import org.apache.impala.analysis.GrantRevokeRoleStmt; import org.apache.impala.analysis.InsertStmt; import org.apache.impala.analysis.OptimizeStmt; +import org.apache.impala.analysis.ParsedStatement; import org.apache.impala.analysis.Parser; import org.apache.impala.analysis.QueryStmt; import org.apache.impala.analysis.ResetMetadataStmt; @@ -2033,7 +2034,10 @@ public class Frontend { // and profiling. try (FrontendProfile.Scope scope = FrontendProfile.createNewWithScope()) { EventSequence timeline = new EventSequence("Query Compilation"); - TExecRequest result = getTExecRequest(planCtx, timeline); + // a wrapper of the getTExecRequest is in the factory so the implementation + // can handle various planner fallback execution logic (e.g. allowing one + // planner, if execution fails, to call a different planner) + TExecRequest result = getTExecRequest(new CompilerFactoryImpl(), planCtx, timeline); timeline.markEvent("Planning finished"); result.setTimeline(timeline.toThrift()); result.setProfile(FrontendProfile.getCurrent().emitAsThrift()); @@ -2302,8 +2306,8 @@ public class Frontend { } } - private TExecRequest getTExecRequest(PlanCtx planCtx, EventSequence timeline) - throws ImpalaException { + private TExecRequest getTExecRequest(CompilerFactory compilerFactory, + PlanCtx planCtx, EventSequence timeline) throws ImpalaException { TQueryCtx queryCtx = planCtx.getQueryContext(); addPlannerToProfile(PLANNER); LOG.info("Analyzing query: " + queryCtx.client_request.stmt + " db: " @@ -2407,7 +2411,7 @@ public class Frontend { String retryMsg = ""; while (true) { try { - req = doCreateExecRequest(planCtx, warnings, timeline); + req = doCreateExecRequest(compilerFactory, planCtx, warnings, timeline); markTimelineRetries(attempt, retryMsg, timeline); break; } catch (InconsistentMetadataFetchException e) { @@ -2702,17 +2706,20 @@ public class Frontend { FrontendProfile.getCurrent().addChildrenProfile(profile); } - private TExecRequest doCreateExecRequest(PlanCtx planCtx, - List<String> warnings, EventSequence timeline) throws ImpalaException { + private TExecRequest doCreateExecRequest(CompilerFactory compilerFactory, + PlanCtx planCtx, List<String> warnings, EventSequence timeline) + throws ImpalaException { TQueryCtx queryCtx = planCtx.getQueryContext(); + // Parse stmt and collect/load metadata to populate a stmt-local table cache - StatementBase stmt = Parser.parse( - queryCtx.client_request.stmt, queryCtx.client_request.query_options); + ParsedStatement parsedStmt = compilerFactory.createParsedStatement(queryCtx); + User user = new User(TSessionStateUtil.getEffectiveUser(queryCtx.session)); StmtMetadataLoader metadataLoader = new StmtMetadataLoader( this, queryCtx.session.database, timeline, user, queryCtx.getQuery_id()); + //TODO (IMPALA-8788): should load table write ids in transaction context. - StmtTableCache stmtTableCache = metadataLoader.loadTables(stmt); + StmtTableCache stmtTableCache = metadataLoader.loadTables(parsedStmt); if (queryCtx.client_request.query_options.isSetDebug_action()) { DebugUtils.executeDebugAction( queryCtx.client_request.query_options.getDebug_action(), @@ -2737,8 +2744,14 @@ public class Frontend { // Analyze and authorize stmt AnalysisContext analysisCtx = new AnalysisContext(queryCtx, authzFactory_, timeline); - AnalysisResult analysisResult = analysisCtx.analyzeAndAuthorize(stmt, stmtTableCache, - authzChecker_.get(), planCtx.compilationState_.disableAuthorization()); + AnalysisResult analysisResult = analysisCtx.analyzeAndAuthorize(compilerFactory, + parsedStmt, stmtTableCache, authzChecker_.get(), + planCtx.compilationState_.disableAuthorization()); + + // need to re-fetch the parsedStatement because analysisResult can rewrite the + // statement. + parsedStmt = analysisResult.getParsedStmt(); + Preconditions.checkState(analysisResult.getException() == null); if (!planCtx.compilationState_.disableAuthorization()) { LOG.info("Analysis and authorization finished."); @@ -2747,7 +2760,6 @@ public class Frontend { } else { LOG.info("Analysis finished."); } - Preconditions.checkNotNull(analysisResult.getStmt()); analysisResult.getAnalyzer().addWarnings(warnings); TExecRequest result = createBaseExecRequest(queryCtx, analysisResult); for (TableName table : stmtTableCache.tables.keySet()) { @@ -2846,7 +2858,7 @@ public class Frontend { analysisResult.getConvertTableToIcebergStmt().toThrift()); return result; } else if (analysisResult.isTestCaseStmt()) { - CopyTestCaseStmt testCaseStmt = ((CopyTestCaseStmt) stmt); + CopyTestCaseStmt testCaseStmt = ((CopyTestCaseStmt) analysisResult.getStmt()); if (testCaseStmt.isTestCaseExport()) { result.setStmt_type(TStmtType.TESTCASE); result.setResult_set_metadata(new TResultSetMetadata(Arrays.asList( @@ -2890,9 +2902,10 @@ public class Frontend { // If unset, set MT_DOP to 0 to simplify the rest of the code. if (!queryOptions.isSetMt_dop()) queryOptions.setMt_dop(0); + Planner planner = new Planner(compilerFactory, analysisResult, queryCtx, timeline); // create TQueryExecRequest TQueryExecRequest queryExecRequest = - getPlannedExecRequest(planCtx, analysisResult, timeline); + getPlannedExecRequest(planner, queryCtx, planCtx, analysisResult, timeline); for (String column : analysisResult.getAnalyzer().joinColumns()) { result.addToJoin_columns(column); @@ -2918,8 +2931,12 @@ public class Frontend { if (analysisResult.isQueryStmt()) { result.stmt_type = TStmtType.QUERY; result.query_exec_request.stmt_type = result.stmt_type; + // use the parsed statement from the analysis result because the + // rewriter may have changed the statement. + ParsedStatement analyzedStmt = analysisResult.getParsedStmt(); // fill in the metadata - result.setResult_set_metadata(createQueryResultSetMetadata(analysisResult)); + result.setResult_set_metadata( + planner.getTResultSetMetadata(analyzedStmt)); } else if (analysisResult.isInsertStmt() || analysisResult.isCreateTableAsSelectStmt()) { // For CTAS the overall TExecRequest statement type is DDL, but the @@ -3106,34 +3123,14 @@ public class Frontend { return finalizeParams; } - /** - * Add the metadata for the result set - */ - private static TResultSetMetadata createQueryResultSetMetadata( - AnalysisResult analysisResult) { - LOG.trace("create result set metadata"); - TResultSetMetadata metadata = new TResultSetMetadata(); - QueryStmt queryStmt = analysisResult.getQueryStmt(); - int colCnt = queryStmt.getColLabels().size(); - for (int i = 0; i < colCnt; ++i) { - TColumn colDesc = new TColumn(); - colDesc.columnName = queryStmt.getColLabels().get(i); - colDesc.columnType = queryStmt.getResultExprs().get(i).getType().toThrift(); - metadata.addToColumns(colDesc); - } - return metadata; - } - /** * Get the TQueryExecRequest and use it to populate the query context */ - private TQueryExecRequest getPlannedExecRequest(PlanCtx planCtx, - AnalysisResult analysisResult, EventSequence timeline) + private TQueryExecRequest getPlannedExecRequest(Planner planner, TQueryCtx queryCtx, + PlanCtx planCtx, AnalysisResult analysisResult, EventSequence timeline) throws ImpalaException { Preconditions.checkState(analysisResult.isQueryStmt() || analysisResult.isDmlStmt() || analysisResult.isCreateTableAsSelectStmt()); - TQueryCtx queryCtx = planCtx.getQueryContext(); - Planner planner = new Planner(analysisResult, queryCtx, timeline); TQueryExecRequest queryExecRequest = createExecRequest(planner, planCtx); if (planCtx.serializeDescTbl()) { queryCtx.setDesc_tbl_serialized( @@ -3523,4 +3520,8 @@ public class Frontend { } } } + + private CompilerFactory getCompilerFactory(PlanCtx ctx) { + return new CompilerFactoryImpl(); + } } diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java index c8571c0b1..c9a6bb217 100644 --- a/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java @@ -47,6 +47,8 @@ import org.apache.impala.rewrite.NormalizeExprsRule; import org.apache.impala.rewrite.SimplifyCastStringToTimestamp; import org.apache.impala.rewrite.SimplifyConditionalsRule; import org.apache.impala.rewrite.SimplifyDistinctFromRule; +import org.apache.impala.service.CompilerFactory; +import org.apache.impala.service.CompilerFactoryImpl; import org.junit.BeforeClass; import org.junit.Test; @@ -95,8 +97,9 @@ public class ExprRewriteRulesTest extends FrontendTestBase { stmt_ = parse(); analysisCtx_ = makeAnalysisContext(); analyzer_ = AnalysisDriverImpl.createAnalyzer(analysisCtx_, makeTableCache(stmt_)); - stmt_.analyze(analyzer_); - return stmt_; + StatementBase stmtBase = (StatementBase) stmt_.getTopLevelNode(); + stmtBase.analyze(analyzer_); + return stmtBase; } @Override diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java index 56e16593d..035160b19 100644 --- a/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java @@ -91,19 +91,20 @@ public class ExprRewriterTest extends AnalyzerTest { public void RewritesOk(String stmt, int expectedNumChanges, int expectedNumExprTrees) throws ImpalaException { // Analyze without rewrites since that's what we want to test here. - StatementBase parsedStmt = (StatementBase) ParsesOk(stmt); + ParsedStatement parsedStmt = ParsesOk(stmt); AnalyzesOkNoRewrite(parsedStmt); + StatementBase stmtBase = (StatementBase) parsedStmt.getTopLevelNode(); exprToTrue_.reset(); - parsedStmt.rewriteExprs(exprToTrue_); + stmtBase.rewriteExprs(exprToTrue_); Assert.assertEquals(expectedNumChanges, exprToTrue_.getNumChanges()); // Verify that the Exprs were actually replaced. trueToFalse_.reset(); - parsedStmt.rewriteExprs(trueToFalse_); + stmtBase.rewriteExprs(trueToFalse_); Assert.assertEquals(expectedNumExprTrees, trueToFalse_.getNumChanges()); // Make sure the stmt can be successfully re-analyzed. - parsedStmt.reset(); + stmtBase.reset(); AnalyzesOkNoRewrite(parsedStmt); } @@ -211,10 +212,11 @@ public class ExprRewriterTest extends AnalyzerTest { private void CheckNumChangesByEqualityDisjunctsToInRule( String stmt, int expectedNumChanges) throws ImpalaException { - StatementBase parsedStmt = (StatementBase) ParsesOk(stmt); + ParsedStatement parsedStmt = ParsesOk(stmt); AnalyzesOkNoRewrite(parsedStmt); ExprRewriter rewriter = new ExprRewriter(EqualityDisjunctsToInRule.INSTANCE); - parsedStmt.rewriteExprs(rewriter); + StatementBase stmtBase = (StatementBase) parsedStmt.getTopLevelNode(); + stmtBase.rewriteExprs(rewriter); Assert.assertEquals(expectedNumChanges, rewriter.getNumChanges()); } diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java index f86bc96d7..b2b50e697 100755 --- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java @@ -45,12 +45,12 @@ public class ParserTest extends FrontendTestBase { * Also asserts that the first select-list expression is of given class. */ public <C extends Expr> Object ParsesOk(String selectStmtSql, Class<C> cl) { - Object parseNode = ParsesOk(selectStmtSql); - if (!(parseNode instanceof SelectStmt)) { + ParsedStatement parseNode = ParsesOk(selectStmtSql); + if (!(parseNode.getTopLevelNode() instanceof SelectStmt)) { fail(String.format("Statement parsed ok but it is not a select stmt: %s", selectStmtSql)); } - SelectStmt selectStmt = (SelectStmt) parseNode; + SelectStmt selectStmt = (SelectStmt) parseNode.getTopLevelNode(); Expr firstExpr = selectStmt.getSelectList().getItems().get(0).getExpr(); // Check the class of the first select-list expression. assertTrue(String.format( @@ -269,7 +269,7 @@ public class ParserTest extends FrontendTestBase { * with the second tableRef because the first one cannot have hints). */ private void TestJoinHints(String stmt, String... expectedHints) { - SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt); + SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt).getTopLevelNode(); Preconditions.checkState(selectStmt.getTableRefs().size() > 1); List<String> actualHints = new ArrayList<>(); assertTrue(selectStmt.getTableRefs().get(0).getJoinHints().isEmpty()); @@ -282,7 +282,7 @@ public class ParserTest extends FrontendTestBase { } private void TestTableHints(String stmt, String... expectedHints) { - SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt); + SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt).getTopLevelNode(); Preconditions.checkState(selectStmt.getTableRefs().size() > 0); List<String> actualHints = new ArrayList<>(); for (int i = 0; i < selectStmt.getTableRefs().size(); ++i) { @@ -294,7 +294,7 @@ public class ParserTest extends FrontendTestBase { } private void TestTableAndJoinHints(String stmt, String... expectedHints) { - SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt); + SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt).getTopLevelNode(); Preconditions.checkState(selectStmt.getTableRefs().size() > 0); List<String> actualHints = new ArrayList<>(); for (int i = 0; i < selectStmt.getTableRefs().size(); ++i) { @@ -312,7 +312,7 @@ public class ParserTest extends FrontendTestBase { * expected hints. */ private void TestSelectListHints(String stmt, String... expectedHints) { - SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt); + SelectStmt selectStmt = (SelectStmt) ParsesOk(stmt).getTopLevelNode(); List<String> actualHints = new ArrayList<>(); List<PlanHint> hints = selectStmt.getSelectList().getPlanHints(); for (PlanHint hint: hints) actualHints.add(hint.toString()); @@ -341,7 +341,8 @@ public class ParserTest extends FrontendTestBase { */ private void TestInsertStmtHints(String pattern, String hint, String... expectedHints) { for (InsertStmt.HintLocation loc: InsertStmt.HintLocation.values()) { - InsertStmt insertStmt = (InsertStmt) ParsesOk(InjectInsertHint(pattern, hint, loc)); + InsertStmt insertStmt = (InsertStmt) ParsesOk( + InjectInsertHint(pattern, hint, loc)).getTopLevelNode(); assertEquals(expectedHints, HintsToStrings(insertStmt.getPlanHints())); } } @@ -360,7 +361,8 @@ public class ParserTest extends FrontendTestBase { * Parses stmt and checks that the CTAS hints stmt are the expected hints. */ private void TestCtasHints(String stmt, String... expectedHints) { - CreateTableAsSelectStmt ctasStmt = (CreateTableAsSelectStmt) ParsesOk(stmt); + CreateTableAsSelectStmt ctasStmt = + (CreateTableAsSelectStmt) ParsesOk(stmt).getTopLevelNode(); assertEquals(expectedHints, HintsToStrings(ctasStmt.getInsertStmt().getPlanHints())); } @@ -1420,7 +1422,7 @@ public class ParserTest extends FrontendTestBase { // Should be parsed as 3 + (3!) // TODO: disabled b/c IMPALA-2149 - low precedence of prefix ! prevents this from // parsing in the expected way - SelectStmt stmt = (SelectStmt) ParsesOk("SELECT 3 + 3!"); + SelectStmt stmt = (SelectStmt) ParsesOk("SELECT 3 + 3!").getTopLevelNode(); Expr e = stmt.getSelectList().getItems().get(0).getExpr(); assertTrue(e instanceof ArithmeticExpr); // ArithmeticExpr ae = (ArithmeticExpr) e; @@ -1432,7 +1434,7 @@ public class ParserTest extends FrontendTestBase { // ((ArithmeticExpr)ae.getChild(1)).getOp()); // Test factorial associativity. - stmt = (SelectStmt) ParsesOk("SELECT 3! = 4"); + stmt = (SelectStmt) ParsesOk("SELECT 3! = 4").getTopLevelNode(); // Should be parsed as (3!) = (4) e = stmt.getSelectList().getItems().get(0).getExpr(); assertTrue(e instanceof BinaryPredicate); @@ -1441,7 +1443,7 @@ public class ParserTest extends FrontendTestBase { assertEquals(2, bp.getChildren().size()); // Test != not broken - stmt = (SelectStmt) ParsesOk("SELECT 3 != 4"); + stmt = (SelectStmt) ParsesOk("SELECT 3 != 4").getTopLevelNode(); // Should be parsed as (3) != (4) e = stmt.getSelectList().getItems().get(0).getExpr(); assertTrue(e instanceof BinaryPredicate); @@ -1700,8 +1702,8 @@ public class ParserTest extends FrontendTestBase { // Test right associativity of NOT. for (String notStr : notStrs) { - SelectStmt stmt = - (SelectStmt) ParsesOk(String.format("select %s a != b", notStr)); + SelectStmt stmt = (SelectStmt) ParsesOk( + String.format("select %s a != b", notStr)).getTopLevelNode(); // The NOT should be applied on the result of a != b, and not on a only. Expr e = stmt.getSelectList().getItems().get(0).getExpr(); assertTrue(e instanceof CompoundPredicate); diff --git a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java index ca076eb37..0f891a515 100644 --- a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java @@ -19,6 +19,8 @@ package org.apache.impala.analysis; import java.util.Arrays; +import org.apache.impala.analysis.ParsedStatement; +import org.apache.impala.analysis.ParsedStatementImpl; import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache; import org.apache.impala.authorization.NoopAuthorizationFactory; import org.apache.impala.authorization.User; @@ -50,7 +52,7 @@ public class StmtMetadataLoaderTest { throws ImpalaException { try (ImpaladTestCatalog catalog = new ImpaladTestCatalog()) { Frontend fe = new Frontend(new NoopAuthorizationFactory(), catalog); - StatementBase stmt = Parser.parse(stmtStr); + ParsedStatement stmt = new ParsedStatementImpl(stmtStr); // Catalog is fresh and no tables are cached. validateUncached(stmt, fe, expectedNumLoadRequests, expectedNumCatalogUpdates, expectedDbs, expectedTables); @@ -62,7 +64,7 @@ public class StmtMetadataLoaderTest { private void testNoLoad(String stmtStr) throws ImpalaException { try (ImpaladTestCatalog catalog = new ImpaladTestCatalog()) { Frontend fe = new Frontend(new NoopAuthorizationFactory(), catalog); - StatementBase stmt = Parser.parse(stmtStr); + ParsedStatement stmt = new ParsedStatementImpl(stmtStr); validateCached(stmt, fe, new String[]{}, new String[]{}); } } @@ -71,7 +73,7 @@ public class StmtMetadataLoaderTest { throws ImpalaException { try (ImpaladTestCatalog catalog = new ImpaladTestCatalog()) { Frontend fe = new Frontend(new NoopAuthorizationFactory(), catalog); - StatementBase stmt = Parser.parse(stmtStr); + ParsedStatement stmt = new ParsedStatementImpl(stmtStr); EventSequence timeline = new EventSequence("Test Timeline"); StmtMetadataLoader mdLoader = new StmtMetadataLoader(fe, Catalog.DEFAULT_DB, timeline); @@ -110,7 +112,7 @@ public class StmtMetadataLoaderTest { } // Assume tables in the stmt are not acid tables. - private void validateUncached(StatementBase stmt, Frontend fe, + private void validateUncached(ParsedStatement stmt, Frontend fe, int expectedNumLoadRequests, int expectedNumCatalogUpdates, String[] expectedDbs, String[] expectedTables) throws InternalException { EventSequence timeline = new EventSequence("Test Timeline"); @@ -129,7 +131,7 @@ public class StmtMetadataLoaderTest { validateTables(stmtTableCache, expectedTables); } - private void validateCached(StatementBase stmt, Frontend fe, + private void validateCached(ParsedStatement stmt, Frontend fe, String[] expectedDbs, String[] expectedTables) throws InternalException { EventSequence timeline = new EventSequence("Test Timeline"); StmtMetadataLoader mdLoader = diff --git a/fe/src/test/java/org/apache/impala/common/FrontendFixture.java b/fe/src/test/java/org/apache/impala/common/FrontendFixture.java index 94f241209..83a02bba6 100644 --- a/fe/src/test/java/org/apache/impala/common/FrontendFixture.java +++ b/fe/src/test/java/org/apache/impala/common/FrontendFixture.java @@ -30,6 +30,8 @@ import org.apache.impala.analysis.ColumnDef; import org.apache.impala.analysis.CreateTableStmt; import org.apache.impala.analysis.CreateViewStmt; import org.apache.impala.analysis.FunctionName; +import org.apache.impala.analysis.ParsedStatement; +import org.apache.impala.analysis.ParsedStatementImpl; import org.apache.impala.analysis.ParseNode; import org.apache.impala.analysis.Parser; import org.apache.impala.analysis.QueryStmt; @@ -53,6 +55,8 @@ import org.apache.impala.catalog.Table; import org.apache.impala.catalog.Type; import org.apache.impala.catalog.View; import org.apache.impala.service.CatalogOpExecutor; +import org.apache.impala.service.CompilerFactory; +import org.apache.impala.service.CompilerFactoryImpl; import org.apache.impala.service.Frontend; import org.apache.impala.testutil.ImpaladTestCatalog; import org.apache.impala.testutil.TestUtils; @@ -255,7 +259,8 @@ public class FrontendFixture { Preconditions.checkNotNull(db, "Test views must be created in an existing db."); // 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) parseStmt(createViewStmt.getInlineViewDef()); + QueryStmt viewStmt = + (QueryStmt) parseStmt(createViewStmt.getInlineViewDef()).getTopLevelNode(); View dummyView = View.createTestView(db, createViewStmt.getTbl(), viewStmt); db.addTable(dummyView); testTables_.add(dummyView); @@ -349,12 +354,12 @@ public class FrontendFixture { /** * Parse 'stmt' and return the root StatementBase. */ - public StatementBase parseStmt(String stmt) { + public ParsedStatement parseStmt(String stmt) { try { - StatementBase node = Parser.parse(stmt); - assertNotNull(node); - return node; - } catch (AnalysisException e) { + ParsedStatement parsedStmt = new ParsedStatementImpl(stmt); + assertNotNull(parsedStmt.getTopLevelNode()); + return parsedStmt; + } catch (ImpalaException e) { fail("Parser error:\n" + e.getMessage()); throw new IllegalStateException(); // Keep compiler happy } @@ -362,13 +367,14 @@ public class FrontendFixture { public AnalysisResult parseAndAnalyze(String stmt, AnalysisContext ctx) throws ImpalaException { - StatementBase parsedStmt = Parser.parse(stmt, ctx.getQueryOptions()); + CompilerFactory compilerFactory = new CompilerFactoryImpl(); + ParsedStatement parsedStmt = new ParsedStatementImpl(stmt, ctx.getQueryOptions()); User user = new User(TSessionStateUtil.getEffectiveUser(ctx.getQueryCtx().session)); StmtMetadataLoader mdLoader = new StmtMetadataLoader( frontend_, ctx.getQueryCtx().session.database, null, user, null); StmtTableCache stmtTableCache = mdLoader.loadTables(parsedStmt); - AnalysisResult analysisResult = ctx.analyzeAndAuthorize(parsedStmt, stmtTableCache, - frontend_.getAuthzChecker()); + AnalysisResult analysisResult = ctx.analyzeAndAuthorize(compilerFactory, parsedStmt, + stmtTableCache, frontend_.getAuthzChecker()); Preconditions.checkState(analysisResult.getException() == null); return analysisResult; } 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 5e7611025..d544b2f65 100644 --- a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java +++ b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java @@ -29,8 +29,11 @@ import java.util.function.Supplier; import org.apache.impala.analysis.AnalysisContext; import org.apache.impala.analysis.AnalysisContext.AnalysisDriverImpl; import org.apache.impala.analysis.AnalysisContext.AnalysisResult; +import org.apache.impala.analysis.AnalysisDriver; import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.InsertStmt; +import org.apache.impala.analysis.ParsedStatement; +import org.apache.impala.analysis.ParsedStatementImpl; import org.apache.impala.analysis.ParseNode; import org.apache.impala.analysis.Parser; import org.apache.impala.analysis.StatementBase; @@ -54,6 +57,8 @@ import org.apache.impala.catalog.Function; import org.apache.impala.catalog.ScalarType; import org.apache.impala.catalog.Table; import org.apache.impala.catalog.Type; +import org.apache.impala.service.CompilerFactory; +import org.apache.impala.service.CompilerFactoryImpl; import org.apache.impala.service.FeCatalogManager; import org.apache.impala.service.Frontend; import org.apache.impala.service.FrontendProfile; @@ -179,7 +184,7 @@ public class FrontendTestBase extends AbstractFrontendTest { /** * Parse 'stmt' and return the root StatementBase. */ - public StatementBase ParsesOk(String stmt) { + public ParsedStatement ParsesOk(String stmt) { return feFixture_.parseStmt(stmt); } @@ -272,15 +277,16 @@ public class FrontendTestBase extends AbstractFrontendTest { /** * Analyzes the given statement without performing rewrites or authorization. */ - public StatementBase AnalyzesOkNoRewrite(StatementBase stmt) throws ImpalaException { + public StatementBase AnalyzesOkNoRewrite(ParsedStatement stmt) throws ImpalaException { try (FrontendProfile.Scope scope = FrontendProfile.createNewWithScope()) { AnalysisContext ctx = createAnalysisCtx(); StmtMetadataLoader mdLoader = new StmtMetadataLoader(frontend_, ctx.getQueryCtx().session.database, null); StmtTableCache loadedTables = mdLoader.loadTables(stmt); Analyzer analyzer = AnalysisDriverImpl.createAnalyzer(ctx, loadedTables); - stmt.analyze(analyzer); - return stmt; + StatementBase stmtBase = (StatementBase) stmt.getTopLevelNode(); + stmtBase.analyze(analyzer); + return stmtBase; } } @@ -343,13 +349,15 @@ public class FrontendTestBase extends AbstractFrontendTest { throws ImpalaException { try (FrontendProfile.Scope scope = FrontendProfile.createNewWithScope()) { ctx.getQueryCtx().getClient_request().setStmt(stmt); - StatementBase parsedStmt = Parser.parse(stmt, ctx.getQueryOptions()); + CompilerFactory compilerFactory = new CompilerFactoryImpl(); + ParsedStatement parsedStmt = + compilerFactory.createParsedStatement(ctx.getQueryCtx()); User user = new User(TSessionStateUtil.getEffectiveUser(ctx.getQueryCtx().session)); StmtMetadataLoader mdLoader = new StmtMetadataLoader( fe, ctx.getQueryCtx().session.database, null, user, null); StmtTableCache stmtTableCache = mdLoader.loadTables(parsedStmt); - AnalysisResult analysisResult = - ctx.analyzeAndAuthorize(parsedStmt, stmtTableCache, fe.getAuthzChecker()); + AnalysisResult analysisResult = ctx.analyzeAndAuthorize(compilerFactory, parsedStmt, + stmtTableCache, fe.getAuthzChecker()); Preconditions.checkState(analysisResult.getException() == null); return analysisResult; } diff --git a/fe/src/test/java/org/apache/impala/common/QueryFixture.java b/fe/src/test/java/org/apache/impala/common/QueryFixture.java index 7d17931e1..bb609b902 100644 --- a/fe/src/test/java/org/apache/impala/common/QueryFixture.java +++ b/fe/src/test/java/org/apache/impala/common/QueryFixture.java @@ -27,6 +27,8 @@ import org.apache.impala.analysis.AnalysisContext; import org.apache.impala.analysis.AnalysisContext.AnalysisResult; import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.Expr; +import org.apache.impala.analysis.ParsedStatement; +import org.apache.impala.analysis.ParsedStatementImpl; import org.apache.impala.analysis.SelectStmt; import org.apache.impala.analysis.SqlParser; import org.apache.impala.analysis.SqlScanner; @@ -34,6 +36,8 @@ import org.apache.impala.analysis.StatementBase; import org.apache.impala.analysis.StmtMetadataLoader; import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache; import org.apache.impala.authorization.NoopAuthorizationFactory; +import org.apache.impala.service.CompilerFactory; +import org.apache.impala.service.CompilerFactoryImpl; import org.apache.impala.testutil.TestUtils; import org.apache.impala.thrift.TQueryCtx; import org.apache.impala.thrift.TQueryOptions; @@ -62,7 +66,7 @@ public class QueryFixture { */ public static class AnalysisFixture extends QueryFixture { protected AnalysisContext analysisCtx_; - protected StatementBase stmt_; + protected ParsedStatement stmt_; private AnalysisResult analysisResult_; public AnalysisFixture(AnalysisSessionFixture analysisFixture, String stmtSql) { @@ -72,13 +76,14 @@ public class QueryFixture { public StatementBase analyze() throws AnalysisException { Preconditions.checkState(analysisCtx_ == null, "Already analyzed"); try { + CompilerFactory compilerFactory = new CompilerFactoryImpl(); stmt_ = parse(); analysisCtx_ = makeAnalysisContext(); - analysisResult_ = analysisCtx_.analyzeAndAuthorize(stmt_, + analysisResult_ = analysisCtx_.analyzeAndAuthorize(compilerFactory, stmt_, makeTableCache(stmt_), session_.frontend().getAuthzChecker()); Preconditions.checkState(analysisResult_.getException() == null); Preconditions.checkNotNull(analysisResult_.getStmt()); - return stmt_; + return (StatementBase) stmt_.getTopLevelNode(); } catch (AnalysisException e) { // Tests may want to test analysis errors, else this exception will // fail the tests; no need to call fail() to accomplish that result. @@ -102,7 +107,7 @@ public class QueryFixture { * Create a table cache for the target database, loading tables * needed for the given statement. */ - protected StmtTableCache makeTableCache(StatementBase stmt) { + protected StmtTableCache makeTableCache(ParsedStatement stmt) { StmtMetadataLoader mdLoader = new StmtMetadataLoader(session_.frontend(), db_, null); try { @@ -114,7 +119,7 @@ public class QueryFixture { } } - public StatementBase statement() { return stmt_; } + public StatementBase statement() { return (StatementBase) stmt_.getTopLevelNode(); } public Analyzer analyzer() { Preconditions.checkState(analysisResult_ != null, "Not yet analyzed"); return analysisResult_.getAnalyzer(); @@ -192,8 +197,8 @@ public class QueryFixture { } public SelectStmt selectStmt() { - Preconditions.checkState(stmt_ != null, "Not yet analyzed"); - return (SelectStmt) stmt_; + Preconditions.checkState(stmt_.getTopLevelNode() != null, "Not yet analyzed"); + return (SelectStmt) stmt_.getTopLevelNode(); } /** @@ -248,13 +253,13 @@ public class QueryFixture { return TestUtils.createQueryContext(db_, user_, queryOptions_); } - public StatementBase parse() { + public ParsedStatement parse() { // TODO: Use the parser class when available SqlScanner input = new SqlScanner(new StringReader(stmtSql_)); SqlParser parser = new SqlParser(input); parser.setQueryOptions(queryOptions_); try { - return (StatementBase) parser.parse().value; + return new ParsedStatementImpl((StatementBase) parser.parse().value); } catch (Exception e) { throw new IllegalStateException(e); }
