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 17:

> Patch Set 17: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/12887/

The following Java test failed.

 (pushd fe && mvn test -Dtest=AuditingTest#TestCreateTable)

This is because I changed the following line in CreateTableAsSelectStmt.java 
from

 "FeDb db = analyzer.getDb(createStmt_.getDb(), Privilege.ANY);"

to

 "FeDb db = analyzer.getDb(createStmt_.getDb(), /* throwIfDoesNotExist */ 
false);".

The former statement would register an access event, but the latter statement 
does not.

 addAccessEvent(new TAccessEvent(dbName, TCatalogObjectType.DATABASE,
        privilege.toString()));

When we call getDb(createStmt_.getDb(), Privilege.ANY) in 
CreateTableAsSelectStmt#analyze(), we would register a privilege request like 
the following, which would fail the Preconditions check proposed in patch set 
17 in that there won't be any privilege request for an AuthorizableTable with 
name being "<db_name>.*".

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

Notice that in BaseAuthorizationChecker#analyze(), we explicitly mention the 
following and a privilege request like the above apparently contradicts the 
code comment.

 // Authorize statements that may produce several hierarchical privilege 
requests.
 // Such a statement always has a corresponding table-level privilege request 
if it
 // has column-level privilege request.

Don't know what else we need to change if we change the code in getDb(String 
dbName, Privilege privilege, boolean throwIfDoesNotExist, boolean 
requireGrantOption) to execute "builder.any().onDb(dbName, dbOwner).build()" 
instead of "builder.any().onAnyColumn(dbName, dbOwner).build()". These 2 
statements seem to register privilege requests with different semantic meaning.


--
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: 17
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: Sat, 10 Jan 2026 06:32:00 +0000
Gerrit-HasComments: No

Reply via email to