ji chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/20508 )
Change subject: IMPALA-12398: Ranger role not exists when altering db/table/view owner to a role ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/20508/3/fe/src/main/java/org/apache/impala/analysis/AnalysisUtils.java File fe/src/main/java/org/apache/impala/analysis/AnalysisUtils.java: http://gerrit.cloudera.org:8080/#/c/20508/3/fe/src/main/java/org/apache/impala/analysis/AnalysisUtils.java@51 PS3, Line 51: checkRole > It is a very good idea,will change as per your suggestion. Hi, Fang-Yu, It is a good idea,However, when I try to implement, I find it is a bit difficult to get AuthorizationManager instance in the analyzer object. for example, in the method analyze(Analyzer analyzer) of class AlterTableOrViewSetOwnerStmt, so it is hard for me to call method checkRole , do you have any idea for this. as an alternative solution ,is it possible to define method checkRole in interface AuthorizationFactory instead of AuthorizationManager instead? thanks in advance. http://gerrit.cloudera.org:8080/#/c/20508/3/fe/src/test/java/org/apache/impala/authorization/CatalogServiceTestCatalogWithRanger.java File fe/src/test/java/org/apache/impala/authorization/CatalogServiceTestCatalogWithRanger.java: http://gerrit.cloudera.org:8080/#/c/20508/3/fe/src/test/java/org/apache/impala/authorization/CatalogServiceTestCatalogWithRanger.java@96 PS3, Line 96: List<RangerRole.RoleMember> roleMemberList = : grantGroups.stream() : .map(new Function<String, RangerRole.RoleMember>() { : @Override : public RangerRole.RoleMember apply(String s) { : RangerRole.RoleMember roleMember = : new RangerRole.RoleMember(s, s.equals(RANGER_ADMIN_USER)); : return roleMember; : } : }) : .collect(Collectors.toList()); : role.setGroups(roleMemberList); > It's not clear to me whether we need to call Role#setGroups() on 'roleMembe Hi, Fang-Yu Rao, In our test cases, the set grantGroups is always empty, the code I added is intended to cover the future test cases which Set grantGroups is not empty, Hope it helps. please let me know if you have more concerns. -- To view, visit http://gerrit.cloudera.org:8080/20508 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2b029bdb90111dbd0eab5189360cc81090225cda Gerrit-Change-Number: 20508 Gerrit-PatchSet: 3 Gerrit-Owner: ji chen <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: ji chen <[email protected]> Gerrit-Comment-Date: Tue, 24 Oct 2023 01:21:45 +0000 Gerrit-HasComments: Yes
