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

Change subject: IMPALA-14507: Register column-level privilege requests for 
INSERT
......................................................................


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23569/14/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/23569/14/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@156
PS14, Line 156:           // The table-level SELECT, INSERT, or CREATE must be 
the first table-level
              :           // request, and it must precede all column-level 
privilege requests.
              :           // We allow the CREATE privilege because in 
CreateTableAsSelectStmt#analyze(),
              :           // we call InsertStmt#analyze(), which in turn 
registers column-level INSERT
              :           // privilege requests.
              :           // We allow the ALL privilege because for the UPSERT 
operation against Kudu
              :           // tables, we set the required privilege to ALL since 
we don't have an UPSERT
              :           // privilege yet. Refer to 
InsertStmt#analyzeTargetTable().
              :           Preconditions.checkState((requests.isEmpty() ||
              :               !(privReq.getAuthorizable().getType() == 
Authorizable.Type.COLUMN)) ||
              :               (requests.get(0).getAuthorizable().getType() == 
Authorizable.Type.TABLE &&
              :                   ALLOWED_HIER_AUTHZ_TABLE_PRIVILEGES.contains(
              :                       requests.get(0).getPrivilege())));
> > Is this really what we want to check here?
The revised condition suggested by me above would actually fail the following 
query.

 create table test_db_10.alltypestiny_id as select id from 
functional.alltypestiny

The reason is that we registered the following privilege request for an 
AuthorizableColumn with table name "*" and column name "*". This could not be 
caught by the current condition in that this is the only AuthorizableColumn 
associated with the table 'test_db_10.*'. The registration was triggered by 
"FeDb db = analyzer.getDb(createStmt_.getDb(), Privilege.ANY)" in 
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java.

 privReq = {PrivilegeRequest@9473}
  authorizable_ = {AuthorizableColumn@9474}
   columnName_ = "*"
   dbName_ = "test_db_10"
   tableName_ = "*"
   ownerUser_ = "admin"
   columns_ = {ArrayList@9475}  size = 0
  privilege_ = {Privilege@7676} "ANY"

To work around this, we could consider adding the following condition.

 (privReq.getAuthorizable().getType() == Authorizable.Type.COLUMN &&
                   !privReq.getAuthorizable().getDbName().equals("*") &&
                   privReq.getAuthorizable().getTableName().equals("*") &&
                   privReq.getAuthorizable().getColumnName().equals("*"))

I will give this a try in the next patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ef61801d3b394c56702b193c250492a62b111df
Gerrit-Change-Number: 23569
Gerrit-PatchSet: 14
Gerrit-Owner: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Wed, 07 Jan 2026 00:48:37 +0000
Gerrit-HasComments: Yes

Reply via email to