Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14400 )

Change subject: IMPALA-9002: Add query option to only check SELECT privilege in 
SHOW TABLES
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc@278
PS5, Line 278: simplify_check_on_show_tables
Do we need a flag for this behavior? Seems like a useful thing to do in any 
case instead of letting the user decide to change the flag.


http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java@796
PS5, Line 796:         PrivilegeRequest privilegeRequest = new 
PrivilegeRequestBuilder(
             :             authzFactory_.getAuthorizableFactory())
             :             .allOf(requiredPrivilege).onAnyColumn(dbName, 
tblName, tableOwner).build();
             :         if (!authzChecker_.get().hasAccess(user, 
privilegeRequest)) {
             :           iter.remove();
             :         }
It looks like this code loops over all the implied actions of the given 
privilege and returns early if any of the action is allowed based on what I see 
in SentryAuthProvider.java

The implied privileges for Privilege.ANY are listed in inefficient order. See 
below from Privilege.java

ANY.implied_ = EnumSet.of(ALL, OWNER, ALTER, DROP, CREATE, INSERT, SELECT, 
REFRESH);

By default, the EnumSet iterator returns the enums in the order they were 
declared. Can we change the order in which we declare the Privilege enums to an 
order from least allowing to most allowing privilege? Or change the 
implementation of getImpliedPrivileges such that it returns privileges in the 
order of SELECT, INSERT, CREATE, ALTER, DROP, OWNER, REFRESH, ALL so that 
privilege checking is more efficient?



--
To view, visit http://gerrit.cloudera.org:8080/14400
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c
Gerrit-Change-Number: 14400
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]>
Gerrit-Comment-Date: Mon, 14 Oct 2019 18:07:44 +0000
Gerrit-HasComments: Yes

Reply via email to