Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11883 )
Change subject: IMPALA-7808: Refactor Analyzer for easier debugging ...................................................................... Patch Set 1: (8 comments) thanks for the change and making it easier to review. mostly minor comments from my end. http://gerrit.cloudera.org:8080/#/c/11883/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11883/1//COMMIT_MSG@37 PS1, Line 37: Testing pls move this testing section above the Change-Id http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java: http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@176 PS1, Line 176: private final Analyzer analyzer; these members should be suffixed with "_" http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@186 PS1, Line 186: Note tem note, you can always include such review-level comments as comments in gerrit. no need for source changes to communicate these things. http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@389 PS1, Line 389: protected did some other class need this protected? http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@617 PS1, Line 617: public why public? http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@659 PS1, Line 659: public why public? http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@757 PS1, Line 757: public void verifyAggregation() throws AnalysisException { why public? http://gerrit.cloudera.org:8080/#/c/11883/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@894 PS1, Line 894: } just to remind myelf... end of inner class. -- To view, visit http://gerrit.cloudera.org:8080/11883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I576c80c4c7a974df226fc91d8903db275069ed52 Gerrit-Change-Number: 11883 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Rogers <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 06 Nov 2018 17:58:40 +0000 Gerrit-HasComments: Yes
