Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9911 )

Change subject: IMPALA-6650: Add fine-grained DROP privilege
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9911/1/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
File fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java:

http://gerrit.cloudera.org:8080/#/c/9911/1/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java@191
PS1, Line 191:         if (privilegeLevel_ != TPrivilegeLevel.ALL &&
> I'm not sure what the behavior of Hive is. I can look into it and create a
Exactly. I also think it makes sense to have INSERT and/or SELECT on SERVER. 
Might be good to confirm with Adam/Steve also.


http://gerrit.cloudera.org:8080/#/c/9911/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9911/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2479
PS3, Line 2479:     withTestFunction(ctx_.catalog, "functional_text_lzo", "f",
I think it's better to follow the existing addTestDb() addTestTable() pattern 
for consistency. We already have the @Test scope for these test 
dbs/tables/functions. It's not clear to me we need a finer-grained scope.

We can talk about changing the pattern for all addTestSomething() methods, but 
let's leave that for later.

In many cases a single db/table/function is used in several tests, so it's not 
clear to me that this pattern produces simpler code in that case (nesting and 
indirection is harder to read).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea1dfb0b117cb1725e9656b9a79a9aebee82b5e4
Gerrit-Change-Number: 9911
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Adam Holley <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Comment-Date: Wed, 04 Apr 2018 04:25:30 +0000
Gerrit-HasComments: Yes

Reply via email to