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 8: (4 comments) In patch set 8, I attempted to address Quanlong's comments and questions. Hopefully I did not miss anything. Let me know if there are additional suggestions. Thanks! http://gerrit.cloudera.org:8080/#/c/23569/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/23569/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@731 PS7, Line 731: .table(authorizable.getTableName()) > Should we set the owner here? Yes. We should set the owner user here. http://gerrit.cloudera.org:8080/#/c/23569/7/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/23569/7/tests/authorization/test_ranger.py@1489 PS7, Line 1489: admin_client.execute("drop database if exists {0} cascade" > Good catch Quanlong! I think you spotted a bug. Done http://gerrit.cloudera.org:8080/#/c/23569/7/tests/authorization/test_ranger.py@1496 PS7, Line 1496: unique_table = unique_name + "_tbl" > How about UPDATE statement on Kudu and Iceberg tables and MERGE statement o The short answer is no. In what follows I provide what privilege requests are registered currently by Apache Impala for those queries mentioned above with concrete examples. For the sake of completeness, I also include UPSERT (against Kudu tables). Assume that we have 1 Kudu table 'kudu_table_01' and 2 Iceberg tables 'iceberg_table_01' and 'iceberg_table_02' that have the same schema (id int, bool_col boolean, string_col string). 1) UPDATE kudu_table_01 SET bool_col = true WHERE id = 1 - SELECT and ALL privileges on the table 'kudu_table_01'. - SELECT privileges on the columns 'id' and 'bool_col'. Those privilege requests are registered during ModifyStmt#analyze() (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java) and it looks like InsertStmt#analyze() is not involved. 2) UPDATE iceberg_table_01 SET bool_col = true WHERE id = 1 - SELECT and ALL privileges on the table 'iceberg_table_01'. - SELECT privileges on all the columns in the table, including the column of 'string_col' even though it's not referenced explicitly in the query. The privilege request for each column is registered because in IcebergUpdateImpl#buildAndValidateSelectExprs() (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/IcebergUpdateImpl.java), we call createSlotRef() for each column in modifyStmt_.table_.getColumns(). Similarly, InsertStmt#analyze() does not seem to be involved. 3) MERGE INTO iceberg_table_02 USING iceberg_table_01 ON iceberg_table_02.id = iceberg_table_01.id WHEN NOT MATCHED THEN INSERT (string_col) VALUES (iceberg_table_01.string_col) - SELECT and ALL privileges on the table 'iceberg_table_02' (destination table). - SELECT privilege on each column in the table 'iceberg_table_02'. - SELECT privilege on the table 'iceberg_table_01' (source table). - SELECT privilege on each column in the table 'iceberg_table_01'. We register the SELECT privilege on each column of the destination table because we call registerSlotRef() for each column in icebergTable_.getColumns() in IcebergMergeImpl#analyze() (https://github.com/apache/impala/tree/master/fe/src/main/java/org/apache/impala/analysis/IcebergMergeImpl.java). We register the SELECT privilege on each column of the source table because we invoke queryStmt_.analyze(analyzer) in IcebergMergeImpl#analyze() where 'queryStmt_' is a SelectStmt. InsertStmt#analyze() does not seem to be involved here either. 4) UPSERT INTO kudu_table_01 (id, string_col) values (1, 'Alice') - ALL privilege on the table. - INSERT privileges on the columns of 'id' and 'string_col'. We register them due to this patch. Note that the ALL privilege is registered in InsertStmt#analyzeTargetTable(). http://gerrit.cloudera.org:8080/#/c/23569/7/tests/authorization/test_ranger.py@1508 PS7, Line 1508: .format(grantee_user)) : > Can we also add tests for partitioned tables and Iceberg tables? Yes we could. Will add them 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: 8 Gerrit-Owner: Fang-Yu Rao <[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: Thu, 20 Nov 2025 00:42:39 +0000 Gerrit-HasComments: Yes
