Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/20221 )
Change subject: IMPALA-11871: Skip permissions loading and check on HDFS if Ranger is enabled ...................................................................... Patch Set 13: (9 comments) Hi all, I have addressed Quanlong's and Yida's comments on patch set 11. Let me know if there are other suggestions. Thanks! http://gerrit.cloudera.org:8080/#/c/20221/11/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/20221/11/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@931 PS11, Line 931: return false; > nit. unnecessary newline Done http://gerrit.cloudera.org:8080/#/c/20221/11/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@3241 PS11, Line 3241: public boolean isParquetTable() { > nit. unnecessary newline Done http://gerrit.cloudera.org:8080/#/c/20221/11/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/20221/11/tests/authorization/test_ranger.py@87 PS11, Line 87: Impala does not throw an AnalysisException > Am I right in saying that if a ranger user doesn't have write permissions, Thanks Yida! You are correct. More precisely, if the Impala service user does not have the write permission on the destination HDFS path corresponding to the destination table or partition, with this patch the query analysis will succeed but we will encounter a runtime exception later due to the Impala service user not having the write permission. Note that usually the authorized user here refers to the requesting user that submits the INSERT query to the Impala service for execution and the requesting user is usually different from the Impala service user. http://gerrit.cloudera.org:8080/#/c/20221/11/tests/authorization/test_ranger.py@89 PS11, Line 89: HDFS permission. > nit: "HDFS permissions" might be more clear. Thanks Quanlong! I will revise the code comment accordingly in the next patch. http://gerrit.cloudera.org:8080/#/c/20221/11/tests/authorization/test_ranger.py@168 PS11, Line 168: _value) : authz_err = "AuthorizationException: User '{0}' does not have privileges > Details of the codes might be stale in the future. We can just comment that Thanks Quanlong! I will revise the code comment accordingly in the next patch. http://gerrit.cloudera.org:8080/#/c/20221/11/tests/authorization/test_ranger.py@173 PS11, Line 173: .format(unique_database), user=ADMIN) > Could you add a test here showing the same LOAD DATA statement fails? Sure. I will add 2 negative tests in the next patch to verify we need the following 2 privileges granted to the requesting user to execute the LOAD DATA statement. I could also add 1 negative test in the previous test test_insert_with_catalog_v1 too. http://gerrit.cloudera.org:8080/#/c/20221/11/tests/authorization/test_ranger.py@197 PS11, Line 197: > Or could you add a test after revoking the privilege? It'd be better to hav Thanks Quanlong! I will add 2 negative tests in the next patch as mentioned in my response above. http://gerrit.cloudera.org:8080/#/c/20221/12/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/20221/12/tests/authorization/test_ranger.py@203 PS12, Line 203: > flake8: E271 multiple spaces after keyword Done http://gerrit.cloudera.org:8080/#/c/20221/12/tests/authorization/test_ranger.py@210 PS12, Line 210: > flake8: E271 multiple spaces after keyword Done -- To view, visit http://gerrit.cloudera.org:8080/20221 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id33c400fbe0c918b6b65d713b09009512835a4c9 Gerrit-Change-Number: 20221 Gerrit-PatchSet: 13 Gerrit-Owner: Halim Kim <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Halim Kim <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 28 May 2024 09:45:04 +0000 Gerrit-HasComments: Yes
