Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12542 )
Change subject: IMPALA-7917 (Part 2): Decouple Sentry from Impala ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/12542/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/12542/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2516 PS2, Line 2516: * the specified privilege. > I think you can shorten this by moving the if statement inside the lambda Done http://gerrit.cloudera.org:8080/#/c/12542/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2694 PS2, Line 2694: builder.grantOption(); > nit: The style is inconsistent with the rest of the file Done http://gerrit.cloudera.org:8080/#/c/12542/2/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java File fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java: http://gerrit.cloudera.org:8080/#/c/12542/2/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java@121 PS2, Line 121: analyzer.registerPrivReq(builder -> > nit: There are a few different lambda styles in different files. Wondering I like the last one. Will update. http://gerrit.cloudera.org:8080/#/c/12542/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableServer.java File fe/src/main/java/org/apache/impala/authorization/AuthorizableServer.java: http://gerrit.cloudera.org:8080/#/c/12542/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableServer.java@27 PS2, Line 27: serverName_ = serverName == null ? "server" : serverName; > Any reason the `null` is handled here instead of being checked in the `Auth Yeah that's how it works AFAIK. Basically if the name is not specified, it will use the default name. For example: "grant select on server to role foo" "grant select on server my_server to role foo" -- To view, visit http://gerrit.cloudera.org:8080/12542 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I051d06451fb14ba2ffb3f7e0d4aae37dee55ab86 Gerrit-Change-Number: 12542 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Austin Nobis (450) Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <prog...@cloudera.com> Gerrit-Comment-Date: Thu, 21 Feb 2019 20:59:33 +0000 Gerrit-HasComments: Yes
