Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15833 )
Change subject: IMPALA-9708: Remove Sentry support ...................................................................... Patch Set 5: Hi Joe, thank you for the effort to remove Sentry support! I do not have any major comment but the following very minor one. After removing Sentry support, there is no role-based authorization provider in Impala, meaning that some statement like "SHOW ROLES" is no longer supported. In this regard, I was wondering whether we also need to remove the related code. For instance, in the case of "SHOW ROLES" described above, we may remove the method of getRoles() at 1) https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/service/JniFrontend.java#L531-L541, 2) https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java#L54, and 3) https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java#L99-L107. Also, some production rule like show_roles_stmt at https://github.com/apache/impala/blob/master/fe/src/main/cup/sql-parser.cup#L944-L951 and ShowRoleStmt at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java may have to be removed as well if we decide to completely remove support for role-based authorization. But on the other hand, it seems better or more flexible to keep those statements for role-based authorization in case we would like to add support for role-based authorization (other than Sentry) some day and I guess that is the reason why you chose not to remove these statements for role-based authorization. -- To view, visit http://gerrit.cloudera.org:8080/15833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e99c15936d6d250cf258e3a1dcba11d3eb4661e Gerrit-Change-Number: 15833 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Mon, 18 May 2020 16:18:57 +0000 Gerrit-HasComments: No
