Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-2836: ADD Partition now only requires INSERT permissions ......................................................................
Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/3327/1//COMMIT_MSG Commit Message: PS1, Line 7: ADD Partition How about DROP or any other alter table statements? Does your change handle these cases as well? Shouldn't we have a consistent behavior across all alter table statements in terms of required permissions? PS1, Line 9: analyse analyze (typo) PS1, Line 14: add partition ADD PARTITION PS1, Line 15: Permissions permissions PS1, Line 15: Alter In general, you don't need to comment on every test case you added. Simply mentioning that you added/modified the authorization tests is sufficient. http://gerrit.cloudera.org:8080/#/c/3327/1/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java: PS1, Line 71: Extra space, please remove. PS1, Line 72: protected void analyzeUsingPrivilege(Analyzer analyzer, Privilege privilege) throws AnalysisException { Long line (lines should be less than 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/3327 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d971f0ab46fed87b0428d344d002a42644b9847 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-HasComments: Yes
