Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10918 )
Change subject: IMPALA-6881: COMPUTE STATS should require SELECT privilege at analysis ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/10918/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10918/3//COMMIT_MSG@10 PS3, Line 10: register the word "register" stuck out. seems like an implementation detail. would it be equivalent to simply replace it with "check"? http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2389 PS3, Line 2389: Privilege... privilege add a comment clarifying whether all or some privilege is required when multiple privs are specified. http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2393 PS3, Line 2393: for (Privilege priv: privilege) { nit: space before ":" (for consistency with this file) http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2408 PS3, Line 2408: for (Privilege priv: privilege) { nit: space before ":" http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/10918/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@1166 PS3, Line 1166: TPrivilegeLevel.ALTER, TPrivilegeLevel.SELECT to avoid unrolling the loop over compute/drop, would it be straightforward to loop over [(op, exp_priv)]? for this case, that would be: [("compute", [TPrivilegeLevel.ALTER, TPrivilegeLevel.SELECT]), ("drop", [TPrivilegeLevel.ALTER])] -- To view, visit http://gerrit.cloudera.org:8080/10918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icead43e689404a286859344e309427eb6c68646a Gerrit-Change-Number: 10918 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Adam Holley <ahol...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Tue, 17 Jul 2018 23:21:04 +0000 Gerrit-HasComments: Yes
