Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/17712 )
Change subject: IMPALA-10806: Create single node plan slowdown when hundreds of inline views are joined ...................................................................... Patch Set 3: (9 comments) http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@9 PS2, Line 9: Creating a single node plan for the following SQL sometime > nit. "Creating a single node plan for the following SQL sometime can slowdo Done http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@16 PS2, Line 16: The reasons for the slow generation of plans are as follows > nit. "are as follows". Done http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@17 PS2, Line 17: 1. Many auxiliary predicates are added to GlobalState.conjuncts causing > nit. "Many auxiliary predicates" Done http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@19 PS2, Line 19: I > nit. In Done http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@29 PS2, Line 29: Testing: > May add some new tests to demonstrate the compilation time reduction. I didn't find the right place to add tests and add a repro query here http://gerrit.cloudera.org:8080/#/c/17712/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/17712/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@391 PS2, Line 391: conjunctsFromQuery = ne > nit. Based on how this map is populated, it may be better to rename the map Done http://gerrit.cloudera.org:8080/#/c/17712/2/fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java File fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java: http://gerrit.cloudera.org:8080/#/c/17712/2/fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java@190 PS2, Line 190: ExprSu > Do we need to handle element not exist exception? Remove this method http://gerrit.cloudera.org:8080/#/c/17712/2/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/17712/2/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1218 PS2, Line 1218: pr> nullableR > nit. If all expressions on RHS are materialized, then this entire trimming I think there is not a way to know the trimming is beneficial in advance, but the cost is negligible compared to ExprSubstitutionMap#compose http://gerrit.cloudera.org:8080/#/c/17712/2/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1209 PS2, Line 1209: if (outputSmap != null) outputSmap.trim(inlineViewRef.getBaseTblSmap(), analyzer); : outputSmap = ExprSubstitutionMap.compose( : outputSmap, rootNode.getOutputSmap(), analyzer); : if (analyzer.isOuterJoined(inlineViewRef.getId())) { : // Exprs against non-matched rows of an outer join should always return NULL. : // Make the rhs exprs of the output smap nullable, if necessary. This expr wrapping : // must be performed on the composed smap, and not on the the inline view's smap, : // because the rhs exprs must first be resolved against the physical output of : // 'planRoot' to correctly determine whether wrapping is necessary. : List<Expr> nullableRhs = TupleIsNullPredicate.wrapExprs( : outputSmap.getRhs(), rootNode.getTupleIds(), analyzer); : outputSmap = new ExprSubstitutionMap(outputSmap.getLhs(), nullableRhs); : } : // Set output smap of rootNode *before* creating a SelectNode for proper resolution. : rootNode.setOutputSmap(outputSmap); : > nit. Wonder if this block of code can be made a new method as ExprSubstitut Done -- To view, visit http://gerrit.cloudera.org:8080/17712 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb4011b6167a0e61438a73c4dba6f1cd0a4e8c6a Gerrit-Change-Number: 17712 Gerrit-PatchSet: 3 Gerrit-Owner: Xianqing He <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Xianqing He <[email protected]> Gerrit-Comment-Date: Tue, 27 Jul 2021 09:17:43 +0000 Gerrit-HasComments: Yes
