Csaba Ringhofer 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: Code-Review+1 (4 comments) The change looks good to me in general, I have some minor comments about tests/comments/preconditions. http://gerrit.cloudera.org:8080/#/c/19194/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19194/7//COMMIT_MSG@16 PS7, Line 16: tables, columns in the database where the UDF belongs to. : > Yes. Your understanding is correct. In Impala, the SELECT privilege on a UD Can you add this info to the commit message? I think the best would to also create a Jira about this difference and mention it near related tests. While this is a small difference, someone may bump into it in the future and would have to investigate it again. Whether we should change this incompatible behavior is a different question. http://gerrit.cloudera.org:8080/#/c/19194/8/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java File fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java: http://gerrit.cloudera.org:8080/#/c/19194/8/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java@352 PS8, Line 352: // Call analyze() to perform path resolution so that 'db_' and 'fn_' of At this point db name cannot be *, right? Can you add a precondition about that? http://gerrit.cloudera.org:8080/#/c/19194/8/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/19194/8/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@53 PS8, Line 53: * nit: unneeded change? http://gerrit.cloudera.org:8080/#/c/19194/8/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/8/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@555 PS8, Line 555: ScalarFunction fn = addFunction("functional", "f"); Can you also add another function, e.g. f2? A test could check the following situation: - the user has an insert or refresh privilege on functional - the user has select privilege on f2, but not on f - the user should not be able to access f -- 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 <fangyu....@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Thu, 24 Nov 2022 17:27:10 +0000 Gerrit-HasComments: Yes
