Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14106 )

Change subject: IMPALA-8228: Ownership support for Ranger authz
......................................................................


Patch Set 12: Code-Review+2

(2 comments)

Thanks for the reviews, carrying +2.

http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizable.java:

http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@65
PS11, Line 65:     int ownerHash = getOwnerUser() == null ? 0 : 
getOwnerUser().hashCode();
             :     int nameHash = getName().hashCode();
             :     return nameHash * 31 + ownerHash;
> nit: You can use Objects.hash(nameHash, ownerHash)
Objects.hash is what I tried but since owner is nullable, it causes an NPE.


http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
File 
fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3068
PS11, Line 3068: String>builder()
> nit: I think Java 8 can infer this without explicitly specifying the type p
Failed to execute goal 
org.apache.maven.plugins:maven-compiler-plugin:3.3:testCompile 
(default-testCompile) on project impala-frontend: Compilation failure
[ERROR] 
/home/bharath/impala/Impala/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java:[3110,15]
 incompatible types: 
com.google.common.collect.ImmutableMap<java.lang.Object,java.lang.Object> 
cannot be converted to 
com.google.common.collect.ImmutableMap<org.apache.impala.authorization.AuthorizationTestBase.AuthzTest,java.lang.String>

Think we don't have this https://github.com/google/guava/issues/1166



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 12
Gerrit-Owner: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Austin Nobis <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Thu, 12 Sep 2019 04:30:50 +0000
Gerrit-HasComments: Yes

Reply via email to