Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/20742 )
Change subject: IMPALA-11501: Add flag to allow catalog-cache operations on masked tables ...................................................................... Patch Set 2: (3 comments) > Patch Set 2: > > (11 comments) Thanks a lot for the detailed explanation Quanlong! I left some very minor comments. http://gerrit.cloudera.org:8080/#/c/20742/1/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/20742/1/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@222 PS1, Line 222: (request.getPrivilege() != Privilege.REFRESH : || !BackendConfig.INSTANCE.allowCatalogCacheOpFromMaskedUsers()) This is just for my own understanding. Is it true that by adding this additional condition we could improve the performance of INVALIDATE METADATA <table> when '--allow_catalog_cache_op_from_masked_users' is true (in both catalog modes), because the following call to db.getTable(authorizableTable.getTableName()) at https://gerrit.cloudera.org/c/20742/1/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#231 is a heavy operation and will be avoided in this case? In addition, according to our discussion at https://gerrit.cloudera.org/c/20742/1/tests/authorization/test_ranger.py#1610, since Impala REFRESH <table> statement in the local catalog mode currently still triggers the loading of table metadata in other place(s), this additional condition could only improve the performance of REFRESH <table> in the legacy catalog mode? http://gerrit.cloudera.org:8080/#/c/20742/1/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/20742/1/tests/authorization/test_ranger.py@1608 PS1, Line 1608: self.execute_query_expect_success( : non_admin_client, : "refresh functional.alltypestiny partition(year=2009, month=1)", user=user) > Nice findings! Thanks a lot for the detailed explanation Quanlong! Things are much clearer after your explanation. :-) Now I understand that CatalogMetaProvider#updateCatalogCache() will be called to invalidate the table metadata after a DDL according to https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java#L1274-L1287. http://gerrit.cloudera.org:8080/#/c/20742/2/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/20742/2/tests/authorization/test_ranger.py@1615 PS2, Line 1615: all Would the REFRESH privilege be sufficient in this case as mentioned in the commit message? -- To view, visit http://gerrit.cloudera.org:8080/20742 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45935654cbf05a55d740f1b04781022c271f7678 Gerrit-Change-Number: 20742 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Thu, 07 Dec 2023 01:31:06 +0000 Gerrit-HasComments: Yes
