Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17184 )
Change subject: IMPALA-10483(part-1): Refactor table mask resolving ...................................................................... Patch Set 1: (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: based nit: 'base' instead of 'based' here and few other places. 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 clauses (SELECT, FROM, WHERE etc.) needs to explicitly call setDoTableMasking(). If there's a clause that is missing this implementation or if the programmer did not invoke this method on a clause for some reason, the flag will remain false and we could end up not applying the table mask. But as long as we have good test coverage on the queries, this may be ok. 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: public void setDoTableMasking(boolean doTableMasking) { I would think this would be an @Override of a corresponding method in base class but the base class does not have this method..is that intentional ? -- 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: 1 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 16 Mar 2021 06:21:43 +0000 Gerrit-HasComments: Yes
