diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java index ea5fa3f4c3..adc639faad 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java @@ -373,7 +373,8 @@ public RelNode genLogicalPlan(ASTNode ast) throws SemanticException { PreCboCtx cboCtx = new PreCboCtx(); //change the location of position alias process here processPositionAlias(ast); - if (!genResolvedParseTree(ast, cboCtx)) { + this.setAST(ast); + if (!genResolvedParseTree(cboCtx)) { return null; } ASTNode queryForCbo = ast; @@ -425,10 +426,13 @@ private static RelOptPlanner createPlanner( @Override @SuppressWarnings("rawtypes") - Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticException { + Operator genOPTree(PlannerContext plannerCtx) throws SemanticException { Operator sinkOp = null; boolean skipCalcitePlan = false; + // Save original AST in case CBO tampers with the contents of ast to guarantee fail-safe behavior. + final ASTNode originalAst = (ASTNode) ParseDriver.adaptor.dupTree(this.getAST()); + if (!runCBO) { skipCalcitePlan = true; } else { @@ -443,14 +447,14 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept // SA. We rely on the fact that CBO ignores the unknown tokens (create // table, destination), so if the query is otherwise ok, it is as if we // did remove those and gave CBO the proper AST. That is kinda hacky. - ASTNode queryForCbo = ast; + ASTNode queryForCbo = this.getAST(); if (cboCtx.type == PreCboCtx.Type.CTAS || cboCtx.type == PreCboCtx.Type.VIEW) { queryForCbo = cboCtx.nodeOfInterest; // nodeOfInterest is the query } Pair canCBOHandleReason = canCBOHandleAst(queryForCbo, getQB(), cboCtx); runCBO = canCBOHandleReason.left; if (queryProperties.hasMultiDestQuery()) { - handleMultiDestQuery(ast, cboCtx); + handleMultiDestQuery(this.getAST(), cboCtx); } if (runCBO) { @@ -481,7 +485,7 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept ASTNode newAST = getOptimizedAST(newPlan); // 1.1. Fix up the query for insert/ctas/materialized views - newAST = fixUpAfterCbo(ast, newAST, cboCtx); + newAST = fixUpAfterCbo(this.getAST(), newAST, cboCtx); // 1.2. Fix up the query for materialization rebuild if (mvRebuildMode == MaterializationRebuildMode.AGGREGATE_REBUILD) { @@ -559,13 +563,13 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept } } } catch (Exception e) { + LOG.error("CBO failed, skipping CBO. ", e); boolean isMissingStats = noColsMissingStats.get() > 0; if (isMissingStats) { LOG.error("CBO failed due to missing column stats (see previous errors), skipping CBO"); this.ctx .setCboInfo("Plan not optimized by CBO due to missing statistics. Please check log for more details."); } else { - LOG.error("CBO failed, skipping CBO. ", e); if (e instanceof CalciteSemanticException) { CalciteSemanticException calciteSemanticException = (CalciteSemanticException) e; UnsupportedFeature unsupportedFeature = calciteSemanticException @@ -611,12 +615,14 @@ else if (!conf.getBoolVar(ConfVars.HIVE_IN_TEST) || isMissingStats runCBO = false; disableJoinMerge = defaultJoinMerge; disableSemJoinReordering = false; + // Make sure originalAst is used from here on. + this.setAST(originalAst); if (reAnalyzeAST) { init(true); prunedPartitions.clear(); // Assumption: At this point Parse Tree gen & resolution will always // be true (since we started out that way). - super.genResolvedParseTree(ast, new PlannerContext()); + super.genResolvedParseTree(new PlannerContext()); skipCalcitePlan = true; } } @@ -633,7 +639,7 @@ else if (!conf.getBoolVar(ConfVars.HIVE_IN_TEST) || isMissingStats } if (skipCalcitePlan) { - sinkOp = super.genOPTree(ast, plannerCtx); + sinkOp = super.genOPTree(); } return sinkOp; diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java index 60bfba826d..90549f9f3a 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java @@ -12264,9 +12264,8 @@ private ASTNode rewriteASTWithMaskAndFilter(TableMask tableMask, ASTNode ast, To } } - boolean genResolvedParseTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticException { + boolean genResolvedParseTree(PlannerContext plannerCtx) throws SemanticException { ASTNode child = ast; - this.ast = ast; viewsExpanded = new ArrayList(); ctesExpanded = new ArrayList(); @@ -12369,7 +12368,12 @@ private void getHintsFromQB(QBExpr qbExpr, List hints) { } } - Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticException { + Operator genOPTree(PlannerContext plannerCtx) throws SemanticException { + // Parameters are not utilized when CBO is disabled. + return genOPTree(); + } + + Operator genOPTree() throws SemanticException { // fetch all the hints in qb List hintsList = new ArrayList<>(); getHintsFromQB(qb, hintsList); @@ -12423,14 +12427,15 @@ private void removeASTChild(ASTNode node) { } @SuppressWarnings("checkstyle:methodlength") - void analyzeInternal(ASTNode ast, Supplier pcf) throws SemanticException { + void analyzeInternal(final ASTNode astToAnalyze, Supplier pcf) throws SemanticException { LOG.info("Starting Semantic Analysis"); - // 1. Generate Resolved Parse tree from syntax tree boolean needsTransform = needsTransform(); //change the location of position alias process here - processPositionAlias(ast); + processPositionAlias(astToAnalyze); PlannerContext plannerCtx = pcf.get(); - if (!genResolvedParseTree(ast, plannerCtx)) { + this.setAST(astToAnalyze); + // 1. Generate Resolved Parse tree from syntax tree + if (!genResolvedParseTree(plannerCtx)) { return; } @@ -12476,7 +12481,7 @@ void analyzeInternal(ASTNode ast, Supplier pcf) throws SemanticE } // 2. Gen OP Tree from resolved Parse Tree - Operator sinkOp = genOPTree(ast, plannerCtx); + Operator sinkOp = genOPTree(plannerCtx); boolean usesMasking = false; if (!unparseTranslator.isEnabled() && @@ -12491,11 +12496,12 @@ void analyzeInternal(ASTNode ast, Supplier pcf) throws SemanticE init(true); //change the location of position alias process here processPositionAlias(rewrittenAST); - genResolvedParseTree(rewrittenAST, plannerCtx); + this.setAST(rewrittenAST); + genResolvedParseTree(plannerCtx); if (this instanceof CalcitePlanner) { ((CalcitePlanner) this).resetCalciteConfiguration(); } - sinkOp = genOPTree(rewrittenAST, plannerCtx); + sinkOp = genOPTree(plannerCtx); } }