Qifan Chen 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 2: (9 comments) Looks good! 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: Create single node plan slowdown in the following form SQL nit. "Creating a single node plan for the following SQL sometime can slowdown" http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@16 PS2, Line 16: The reasons for the slow generation of plans are nit. "are as follows". http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@17 PS2, Line 17: 1. auxiliary predicates are added to GlobalState.conjuncts causing nit. "Many auxiliary predicates" http://gerrit.cloudera.org:8080/#/c/17712/2//COMMIT_MSG@19 PS2, Line 19: i nit. In 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. 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: conjunctsWithoutAuxExpr nit. Based on how this map is populated, it may be better to rename the map as conjunctsFromQuery. 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: remove Do we need to handle element not exist exception? 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: (!analyzer.ge nit. If all expressions on RHS are materialized, then this entire trimming operation is a no-op and could be expensive. Is there a way to know the trimming is beneficial in advance? 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) { : // Remove expressions from outputSmap that are not used according to baseSmap, : // in order to optimize the performance of ExprSubstitutionMap#compose : ExprSubstitutionMap baseSmap = inlineViewRef.getBaseTblSmap(); : Preconditions.checkState(outputSmap.size() == baseSmap.size()); : for (int i = outputSmap.size() - 1; i >= 0; --i) { : List<SlotId> slotIds = new ArrayList<>(); : baseSmap.getRhs().get(i).getIds(null, slotIds); : for (SlotId id: slotIds) { : if (!analyzer.getSlotDesc(id).isMaterialized()) { : outputSmap.remove(i); : break; : } : } : } : } nit. Wonder if this block of code can be made a new method as ExprSubstituteMap::trim(ExprSubstitutionMap baseTblSMap). -- 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: 2 Gerrit-Owner: Xianqing He <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Comment-Date: Mon, 26 Jul 2021 14:57:01 +0000 Gerrit-HasComments: Yes
