Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
......................................................................


Patch Set 40:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2903
PS38, Line 2903:
adding


http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2904
PS38, Line 2904:         filter.setPrincipal_id(owner.getId());
it also add a user if specifying a user and that user does not exist.


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1341
PS40, Line 1341: updateOwnerPrivileges
if this is *not* done for drop, what is the harm in waiting for the next update 
from Sentry?


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1493
PS40, Line 1493:       if (table.getMetaStoreTable() != null) {
same question here (why do this for drop).


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2218
PS40, Line 2218:       authzCatalog_.addRole("foo_owner");
confused by this one... after this line, "foo_owner" role exists so the 
subsequent tests do not test the missing role case. Is the comment wrong or is 
something else off?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley <[email protected]>
Gerrit-Reviewer: Adam Holley <[email protected]>
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Comment-Date: Thu, 13 Sep 2018 20:31:59 +0000
Gerrit-HasComments: Yes

Reply via email to