Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16199 )
Change subject: IMPALA-7001: Fix Privilege inconsistency between SHOW TABLES and SHOW FUNCTIONS ...................................................................... Patch Set 2: Code-Review+1 Hi Adam, thanks for working on this patch! The patch looks good to me since you have implemented what Fredy had suggested at https://issues.apache.org/jira/browse/IMPALA-7001. I only have two minor comments regarding the test and the commit message. Specifically, after your patch, a user granted only the privilege of CREATE on a specified database, e.g., functional, would be able to execute a statement like "SHOW FUNCTIONS IN functional", since according to https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/Privilege.java and https://github.com/apache/impala/blob/3a6022ce80ca1cedb629400b18caaf0d1f54137c/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L431-L453, such a statement would succeed as long as the user is granted any privilege in the set {ALL, OWNER, ALTER, DROP, CREATE, INSERT, SELECT, REFRESH}. Before your patch, in order for the statement above to succeed, a user has to be granted any privilege in the set {INSERT, SELECT, REFRESH}. Thus I think it would be good to add one more test case in https://github.com/apache/impala/blob/master/tests/authorization/test_ranger.py, where we 1) grant the privilege of CREATE to a user (as admin_client), and 2) execute a statement like "SHOW FUNCTIONS IN unique_database" to verify there is no exception thrown. On the other hand, I think it may also be good to provide more detail of the difference before and after the patch. For instance, we could mention that a user granted only the privilege of CREATE is now able to execute that SQL statement above after this patch, making it easier for the user to manage the functions it creates. -- To view, visit http://gerrit.cloudera.org:8080/16199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ae7546c206daaf98ecc3de449069027c43c6e1a Gerrit-Change-Number: 16199 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Tamas <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 15 Jul 2020 23:27:59 +0000 Gerrit-HasComments: No
