impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bikramjeet Vig (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2836: Change permissions for ADD/DROP PARTITION
Date Wed, 15 Jun 2016 00:35:32 GMT
Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-2836: Change permissions for ADD/DROP PARTITION
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3327/3//COMMIT_MSG
Commit Message:

PS3, Line 14: ALTER TABLE ADD PARTITION SET LOCATION 'hdfs_path_of_directory'
> There is ALTER TABLE ADD PARTITION LOCATION and ALTER TABLE (PARTITION SPEC
Done


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableAddPartitionStmt.java:

PS3, Line 84: if (location_ != null) {
            :       partitionSpec_.setPrivilegeRequirement(Privilege.ALTER);
            :       location_.analyze(analyzer, Privilege.ALL, FsAction.READ_WRITE);
            :     } else {
            :       partitionSpec_.setPrivilegeRequirement(Privilege.INSERT);
            :     }
> This is confusing to me. Why does the partition spec get a different privil
As mentioned in the commit message, when the partition location is specified in the query,
we require ALTER permissions


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableDropPartitionStmt.java:

PS3, Line 73: INSERT
> Please correct me if I am wrong, but I thought the DROP will still require 
In order to closely resemble how hive handles permissions, this was done. Hive requires DELETE
permissions for this but we do not support fine grained permissions and instead couple up
DELETE permissions with INSERT.


http://gerrit.cloudera.org:8080/#/c/3327/3/fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/AlterTableStmt.java:

PS3, Line 72: analyzeUsingPrivilege
> Let's keep the name "analyze" for the function. Also, you may want to add a
Done


-- 
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: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message