Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/19252 )
Change subject: IMPALA-11728: Set fallback database for functions ...................................................................... Patch Set 19: Code-Review+1 (2 comments) Thanks for the effort Xiaoqing and Csaba! I only have 2 minor comments and I do not have any other suggestion. http://gerrit.cloudera.org:8080/#/c/19252/15/fe/src/main/java/org/apache/impala/analysis/FunctionName.java File fe/src/main/java/org/apache/impala/analysis/FunctionName.java: http://gerrit.cloudera.org:8080/#/c/19252/15/fe/src/main/java/org/apache/impala/analysis/FunctionName.java@128 PS15, Line 128: ltinsDb.NAME; > Yes, the query option doesn't take effect to CREATE/DROP FUNCTION statement Thanks for the confirmation! We probably don't have to consider the CREATE/DROP FUNCTION statements since this use case may not be that common. In that case, could we add 2 more code comments as described in the following if they are reasonable? 1. A code comment for the else-if branch pointing out that this branch is for the SELECT statement with the query option of 'FALLBACK_DB_FOR_FUNCTIONS' being set because for the SELECT statement, 'preferBuiltinsDb' is true. 2. A code comment for the else branch pointing out that this branch is for a) the SELECT statement without the query option of 'FALLBACK_DB_FOR_FUNCTIONS' being set, or b) the CREATE/DROP FUNCTION statements whether or not 'FALLBACK_DB_FOR_FUNCTIONS' is specified. http://gerrit.cloudera.org:8080/#/c/19252/19/fe/src/main/java/org/apache/impala/analysis/FunctionName.java File fe/src/main/java/org/apache/impala/analysis/FunctionName.java: http://gerrit.cloudera.org:8080/#/c/19252/19/fe/src/main/java/org/apache/impala/analysis/FunctionName.java@146 PS19, Line 146: FeDb feDb = analyzer.getDb(dbName, Privilege.VIEW_METADATA, false); Could we add a code comment here pointing out that to execute a UDF of the fallback database in a SELECT statement, the requesting user has be to granted any one of the INSERT, REFRESH, SELECT privileges on the fallback database? -- To view, visit http://gerrit.cloudera.org:8080/19252 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37b7e126718fea1c50723cacbaed898b20bb55e5 Gerrit-Change-Number: 19252 Gerrit-PatchSet: 19 Gerrit-Owner: Xiaoqing Gao <[email protected]> Gerrit-Reviewer: Anonymous Coward <[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-Reviewer: Xiaoqing Gao <[email protected]> Gerrit-Comment-Date: Fri, 02 Dec 2022 04:25:59 +0000 Gerrit-HasComments: Yes
