Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/service/Frontend.java@783 PS6, Line 783: TODO mind filing a JIRA and putting the number here? http://gerrit.cloudera.org:8080/#/c/14106/6/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/6/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3060 PS6, Line 3060: authorize("select count(*) from functional.alltypes") nit: might make this test a little easier to read to do something like create an Map: ImmutableMap.builder() .put("select count(*) from functional.alltypes", selectError("functional.alltypes")) .put("select id from functional.alltypes", selectError("functional.alltypes")) ... .build(); and then you can loop over the queries in the before/after case below to avoid having to duplicate all the queries twice http://gerrit.cloudera.org:8080/#/c/14106/6/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/14106/6/tests/authorization/test_ranger.py@665 PS6, Line 665: # Run show tables on the db. The resulting list should be empty. This happens : # because the created table's ownership information is not aggresively cached : # by the current Catalog implementations. Hence the analysis pass does not : # have access to the ownership information to verify if the current session : # user is actually the owner. We need to fix this by caching the HMS metadata : # more aggressively when the table loads. this is true only in localcatalog, right? otherwise the 'create table' call would have fully populated the cache rather than leaving an incomplete table? http://gerrit.cloudera.org:8080/#/c/14106/6/tests/authorization/test_ranger.py@669 PS6, Line 669: We need to fix this by caching the HMS metadata : # more aggressively when the table loads. add a TODO(IMPALA-xxxxx) with jira -- 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: 6 Gerrit-Owner: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Austin Nobis <ano...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fre...@apache.org> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 10 Sep 2019 23:49:33 +0000 Gerrit-HasComments: Yes
