Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13074 )
Change subject: IMPALA-8280, IMPALA-8281: Add support for show grant user/group with Ranger ...................................................................... Patch Set 9: (4 comments) http://gerrit.cloudera.org:8080/#/c/13074/9/fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java File fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java: http://gerrit.cloudera.org:8080/#/c/13074/9/fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java@65 PS9, Line 65: if (principal_ == null) throw new AnalysisException(String.format("%s '%s' " + : "does not exist.", Principal.toString(principalType_), name_)); since this spans more than one line, can you use {} http://gerrit.cloudera.org:8080/#/c/13074/9/fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java: http://gerrit.cloudera.org:8080/#/c/13074/9/fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java@26 PS9, Line 26: nit: remove an extra empty new line http://gerrit.cloudera.org:8080/#/c/13074/9/fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java@28 PS9, Line 28: nit: remove an extra empty new line http://gerrit.cloudera.org:8080/#/c/13074/9/fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java@30 PS9, Line 30: public static Map<String, String> createColumnResource(TPrivilege privilege) { : Map<String, String> resource = new HashMap<>(); : : resource.put(RangerImpalaResourceBuilder.DATABASE, getOrAll(privilege.getDb_name())); : resource.put(RangerImpalaResourceBuilder.TABLE, getOrAll(privilege.getTable_name())); : resource.put(RangerImpalaResourceBuilder.COLUMN, : getOrAll(privilege.getColumn_name())); : : return resource; : } : : public static Map<String, String> createUriResource(TPrivilege privilege) { : Map<String, String> resource = new HashMap<>(); : String uri = privilege.getUri(); : resource.put(RangerImpalaResourceBuilder.URL, uri == null ? "*" : uri); : : return resource; : } : : public static Map<String, String> createFunctionResource(TPrivilege privilege) { : Map<String, String> resource = new HashMap<>(); : : resource.put(RangerImpalaResourceBuilder.DATABASE, getOrAll(privilege.getDb_name())); : resource.put(RangerImpalaResourceBuilder.UDF, "*"); : : return resource; : } : : private static String getOrAll(String resource) { : return (resource == null) ? "*" : resource; : } add javadoc for public methods -- To view, visit http://gerrit.cloudera.org:8080/13074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic46fb9fc36c9e11ec78d5840d22eb0668150c2a4 Gerrit-Change-Number: 13074 Gerrit-PatchSet: 9 Gerrit-Owner: Austin Nobis <ano...@cloudera.com> Gerrit-Reviewer: Austin Nobis <ano...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 30 Apr 2019 15:19:54 +0000 Gerrit-HasComments: Yes
