Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18050 )
Change subject: [WIP] IMPALA-10992 Planner changes for estimate peak memory - v2 ...................................................................... Patch Set 6: (6 comments) Need to discuss why we can't come up with estimates without generating the distributed plan. It would be better to avoid re-planning and the related state management issues. http://gerrit.cloudera.org:8080/#/c/18050/6/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/18050/6/common/thrift/ImpalaService.thrift@710 PS6, Line 710: // Indicates whether to relan. typo relan http://gerrit.cloudera.org:8080/#/c/18050/6/common/thrift/Query.thrift File common/thrift/Query.thrift: http://gerrit.cloudera.org:8080/#/c/18050/6/common/thrift/Query.thrift@564 PS6, Line 564: // distributed plan among several executor groups. The returned plan satisfies change "several" to "different-sized" http://gerrit.cloudera.org:8080/#/c/18050/6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: http://gerrit.cloudera.org:8080/#/c/18050/6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@1192 PS6, Line 1192: resultExprsBacked_ = Expr.cloneList(resultExprs_); Why is cloning necessary? http://gerrit.cloudera.org:8080/#/c/18050/6/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@1199 PS6, Line 1199: resultExprs_ = Expr.cloneList(resultExprsBacked_); Restore should be able to assign/clear rather than clone. http://gerrit.cloudera.org:8080/#/c/18050/6/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java: http://gerrit.cloudera.org:8080/#/c/18050/6/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@104 PS6, Line 104: protected List<Expr> resultExprsBacked_ = null; rather than backup/restore, it may be a little cleaner to have an immutable list that is cloned and modified if needed. http://gerrit.cloudera.org:8080/#/c/18050/6/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: http://gerrit.cloudera.org:8080/#/c/18050/6/fe/src/main/java/org/apache/impala/planner/Planner.java@359 PS6, Line 359: str.append( please remove reformatting of otherwise unchanged code. -- To view, visit http://gerrit.cloudera.org:8080/18050 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If8a31a574b364f39b049a4bae33a8b98c5fc20bd Gerrit-Change-Number: 18050 Gerrit-PatchSet: 6 Gerrit-Owner: Qifan Chen <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Comment-Date: Thu, 02 Dec 2021 18:39:57 +0000 Gerrit-HasComments: Yes
