You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoram Thanga (Code Review)" <ge...@cloudera.org> on 2018/07/02 20:50:37 UTC

[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB

Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/10842 )

Change subject: IMPALA-6086: Use of permanent function should require SELECT privilege on DB
......................................................................


Patch Set 1:

(3 comments)

Abandoning this patch, and raising a new one instead.

http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2576
PS1, Line 2576:       AuthzOk(ctx, "create function default.to_lower(string) returns string " +
> This doesn't actually create a function in the catalog. You need to do the 
Thanks for the suggestion. I did not intent for the function to be reused across test cases, and hence adding to the catalog is probably not required. I've eliminated this step altogether in the new patch.


http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2578
PS1, Line 2578: ToUpper
> nit: it's probably better to call the function to_upper since the actual fu
Oops. Thanks for catching that. I meant to use ToLower and c/p'd the wrong line.


http://gerrit.cloudera.org:8080/#/c/10842/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2596
PS1, Line 2596:     AuthzError(ctx1, "select default.to_lower('ABCDEF')",
> Due to the way authorization works to prefer AuthorizationException over An
Please see above response. I've botched the rebase on top of https://gerrit.cloudera.org/#/c/10841/, so I am going to abandon this change and raise a new one instead.



-- 
To view, visit http://gerrit.cloudera.org:8080/10842
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45f5b51363afbe46653b131fad9bf68e3e3ce71f
Gerrit-Change-Number: 10842
Gerrit-PatchSet: 1
Gerrit-Owner: Zoram Thanga <zo...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Jul 2018 20:50:37 +0000
Gerrit-HasComments: Yes