Quanlong Huang 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 3:

(4 comments)

Somehow I missed the previous comments. Sorry to be late here.

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: oves the performance of "INVALIDATE METADATA <table>" 
statements. For REFRESH
             :     // statements, the performance doesn't differ a lot since 
there are ot
> I am trying to answer my questions above.
Sorry to be late here. Yes, exactly. This avoids getTable() triggering full 
table loading in local catalog mode. I left a comment in the JIRA about this: 
https://issues.apache.org/jira/browse/IMPALA-11501?focusedCommentId=17770339&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17770339

I'll add some comments to make this more clear.


http://gerrit.cloudera.org:8080/#/c/20742/2/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/20742/2/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java@81
PS2, Line 81:   Pair<Table, TableMetaRef> getTableIfPresent(String dbName, 
String tableName);
> Should we add this method to CatalogdMetaProvider.java only? I am asking th
catalog_.getMetaProvider() returns a MetaProvider. Adding this only in 
CatalogdMetaProvider requires adding type checks after it. The current way 
seems more clean.


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, "describe functional.alltypestiny", 
user=user)
              :       # Verify catalogd loads metadata of this table
> Thanks a lot for the detailed explanation Quanlong! Things are much clearer
Ack


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: ida
> Would the REFRESH privilege be sufficient in this case as mentioned in the
Yeah, just wanted to use a short grant statement. I can change it to the 
minimal one.



-- 
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: 3
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: Mon, 11 Dec 2023 08:02:27 +0000
Gerrit-HasComments: Yes

Reply via email to