Fang-Yu Rao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21940 )

Change subject: IMPALA-12554: Create one Ranger policy for multi-column GRANT
......................................................................


Patch Set 1:

(2 comments)

Hi all, I have addressed Daniel's and Csaba's comments. Let me know if there is 
still any additional suggestion. Thanks!

http://gerrit.cloudera.org:8080/#/c/21940/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:

http://gerrit.cloudera.org:8080/#/c/21940/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@327
PS1, Line 327:       LOG.debug("Granting privilege(s) took {} ms",
> This wording suggests that privileges were indeed granted, but we may arriv
Thanks Daniel!

I will revise the message accordingly in the next patch.


http://gerrit.cloudera.org:8080/#/c/21940/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@498
PS1, Line 498: the fields other than database, table, and
             :    * column names are the same.
> Is it actually possible for a single call to contain multiple databases/tab
Thanks Csaba!

It looks like currently Impala does not support multiple databases/tables in a 
single GRANT/REVOKE statement.

I took a look at 
https://github.com/apache/impala/blob/master/fe/src/main/cup/sql-parser.cup and 
found that currently in a single GRANT/REVOKE statement, at most one database 
or one table would be involved.

This is because a) 'privilege_spec' only appears one time on the right-hand 
side of 'grant_privilege_stmt' and 'revoke_privilege_stmt', and b) a 
'privilege_spec' could lead to at most one KW_DATABASE or one KW_TABLE.

In the future if we add the support for including columns in different tables 
that are possibly in different databases in a single GRANT/REVOKE statement, 
then we could add more test cases around this.



--
To view, visit http://gerrit.cloudera.org:8080/21940
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0ebba256c7135b4b0d2160856202292d720c6d
Gerrit-Change-Number: 21940
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Daniel Becker <[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: Thu, 17 Oct 2024 21:51:00 +0000
Gerrit-HasComments: Yes

Reply via email to