Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17184 )
Change subject: IMPALA-10483(part-1): Refactor table mask resolving ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/17184/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17184/1//COMMIT_MSG@10 PS1, Line 10: base > nit: 'base' instead of 'based' here and few other places. Done http://gerrit.cloudera.org:8080/#/c/17184/1//COMMIT_MSG@23 PS1, Line 23: This patch refactor the table mask resolving logic to not apply table > One effect of not applying table mask by default is that now each of the cl Yeah, I think it's a trade-off. Before this patch we apply table mask by default. Then we need to turn it off in many other places, e.g. in analyze() invoked by statement rewriters. If the programmer forgets to turn it off when adding a new rewriter, we may have duplicate or infinite recursive masks on the table. Agree that test coverage are critical for both side. http://gerrit.cloudera.org:8080/#/c/17184/1/fe/src/main/java/org/apache/impala/analysis/WithClause.java File fe/src/main/java/org/apache/impala/analysis/WithClause.java: http://gerrit.cloudera.org:8080/#/c/17184/1/fe/src/main/java/org/apache/impala/analysis/WithClause.java@66 PS1, Line 66: @Override > I would think this would be an @Override of a corresponding method in base Yeah, I'm hesitated on this. The base class (StmtNode) is very clean that only has an analyze() method. So I try to limit the changes of this patch. Added it to StmtNode since it causes confusion to you. -- To view, visit http://gerrit.cloudera.org:8080/17184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia191928fb179b0b0632235c1fff4c18647e5802f Gerrit-Change-Number: 17184 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 16 Mar 2021 09:37:16 +0000 Gerrit-HasComments: Yes
