Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14203 )
Change subject: IMPALA-8933: Enforce ranger deny policy ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/14203/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14203/2//COMMIT_MSG@9 PS2, Line 9: This patch fixes a case where access to a given column is allowed at the table nit: Limit commit message line widths to 72 chars. http://gerrit.cloudera.org:8080/#/c/14203/2//COMMIT_MSG@16 PS2, Line 16: - Manually tested with table level allow and column level deny policies in No luck with automating it in AuthorizationStmtTest? http://gerrit.cloudera.org:8080/#/c/14203/2/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/14203/2/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@234 PS2, Line 234: Trailing white space http://gerrit.cloudera.org:8080/#/c/14203/2/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@235 PS2, Line 235: if (hasTableSelectPriv && How about ALTER privilege? Alter table drop/rename col etc? In-general, shouldn't we just skip 'hasTableSelectPriv' for col privileges and let Ranger/Sentry do its check via hasAccess()? http://gerrit.cloudera.org:8080/#/c/14203/2/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@236 PS2, Line 236: request.getPrivilege() != Privilege.SELECT && 4 spaced indents.https://cwiki.apache.org/confluence/display/IMPALA/Impala+Style+Guide -- To view, visit http://gerrit.cloudera.org:8080/14203 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic60786cd81080feeb0bfcd92aa2be646ee8cb7da Gerrit-Change-Number: 14203 Gerrit-PatchSet: 2 Gerrit-Owner: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 10 Sep 2019 18:50:44 +0000 Gerrit-HasComments: Yes
