Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/19194 )
Change subject: IMPALA-10986: Require the SELECT privilege to execute a UDF ...................................................................... Patch Set 8: (4 comments) Hi all, I have addressed Csaba's comments in the previous review. Let me know if there is any additional suggestion. Thanks very much for the help! http://gerrit.cloudera.org:8080/#/c/19194/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19194/7//COMMIT_MSG@7 PS7, Line 7: execute > Thanks! Can you mention in the commit message that only udf execution privi Sure. Will revise the commit message in my next patch. http://gerrit.cloudera.org:8080/#/c/19194/7//COMMIT_MSG@16 PS7, Line 16: tables, columns in the database where the UDF belongs to. : > Thanks for the investigation! Yes. Your understanding is correct. In Impala, the SELECT privilege on a UDF alone is not sufficient to execute the UDF, whereas in Hive the SELECT privilege on the UDF is sufficient. http://gerrit.cloudera.org:8080/#/c/19194/7//COMMIT_MSG@34 PS7, Line 34: > HMS seems to have an owner field for functions - I don't know whether if af Taking a brief look at the places in FunctionCallExpr#analyzeImpl() where we retrieve the Function object, e.g., https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java#L661, I found that we do not have a field holding the ownership information in the class Function at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/Function.java. On the other hand, the ownership information of a table could be retrieved in Analyzer#getTable() at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/Analyzer.java#L3357. Under the covers the ownership information is available in the underlying org.apache.hadoop.hive.metastore.api.Table. I guess it may require some work to support the OWNER privilege for UDFs in Impala. In the regard, I created IMPALA-11743. http://gerrit.cloudera.org:8080/#/c/19194/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/19194/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@572 PS7, Line 572: onDat > I meant the case when there is a select privilege on functional.f, but only Thanks for the clarification Csaba! Yes. In this regard I will add the following 3 test cases in my next patch. // We do not have to explicitly grant the SELECT privilege on the UDF if we // already grant the SELECT privilege on the database where the UDF belongs // in that granting the SELECT privilege on the database also implies granting // the SELECT privilege on all the UDF's in the database. .ok(onDatabase("functional", TPrivilegeLevel.SELECT)) .ok(onUdf("functional", "f", TPrivilegeLevel.SELECT), onDatabase("functional", TPrivilegeLevel.INSERT)) .ok(onUdf("functional", "f", TPrivilegeLevel.SELECT), onDatabase("functional", TPrivilegeLevel.REFRESH)) -- To view, visit http://gerrit.cloudera.org:8080/19194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5e58ba30545ce169786aac279b00c8f6e09ae740 Gerrit-Change-Number: 19194 Gerrit-PatchSet: 8 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[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, 24 Nov 2022 08:05:18 +0000 Gerrit-HasComments: Yes
