You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Szehon Ho <sz...@cloudera.com> on 2014/04/11 02:49:33 UTC

Review Request 20243: HIVE-6891 - Alter rename partition Perm inheritance and general partition/table owner inheritance

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20243/
-----------------------------------------------------------

Review request for hive.


Bugs: HIVE-6891
    https://issues.apache.org/jira/browse/HIVE-6891


Repository: hive-git


Description
-------

This is a follow-up of HIVE-6648.

Extending the fix to other partition/table operations as well, by refactoring the fixed code in HIVE-6648 into a common FileUtils helper method, and then using it for all table/partition directory creation operation, when "hive.warehouse.subdir.inherit.perms" flag is set.  

Another part of this change is to add ownership inheritance in this code as well when creating directories.  Ownership was already inherited for data (HIVE-3756), but not at the table/partitioned-table level.


Diffs
-----

  common/src/java/org/apache/hadoop/hive/common/FileUtils.java ad82f62 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java f1c7b7b 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 8345d70 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c62e085 
  metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java f731dab 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2559e0e 

Diff: https://reviews.apache.org/r/20243/diff/


Testing
-------

Extending the unit test TestFolderPermission to handle all the new cases of directory creation (create table, external table, static partition, dynamic partition, rename partition).

Unfortunately due to the test using the local file system, I cannot add the ownership inheritance to unit-testing.  I can probably look into using MiniDFS for that, in a follow-up JIRA.


Thanks,

Szehon Ho


Re: Review Request 20243: HIVE-6891 - Alter rename partition Perm inheritance and general partition/table owner inheritance

Posted by Szehon Ho <sz...@cloudera.com>.

> On April 11, 2014, 12:52 a.m., Brock Noland wrote:
> > I don't think we will be able to set owner since we are not superuer? But we can set and should set group.

Thanks I'll make the change, and try on a cluster to verify chgrp will work.


- Szehon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20243/#review40113
-----------------------------------------------------------


On April 11, 2014, 12:49 a.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20243/
> -----------------------------------------------------------
> 
> (Updated April 11, 2014, 12:49 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6891
>     https://issues.apache.org/jira/browse/HIVE-6891
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> This is a follow-up of HIVE-6648.
> 
> Extending the fix to other partition/table operations as well, by refactoring the fixed code in HIVE-6648 into a common FileUtils helper method, and then using it for all table/partition directory creation operation, when "hive.warehouse.subdir.inherit.perms" flag is set.  
> 
> Another part of this change is to add ownership inheritance in this code as well when creating directories.  Ownership was already inherited for data (HIVE-3756), but not at the table/partitioned-table level.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java ad82f62 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java f1c7b7b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 8345d70 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c62e085 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java f731dab 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2559e0e 
> 
> Diff: https://reviews.apache.org/r/20243/diff/
> 
> 
> Testing
> -------
> 
> Extending the unit test TestFolderPermission to handle all the new cases of directory creation (create table, external table, static partition, dynamic partition, rename partition).
> 
> Unfortunately due to the test using the local file system, I cannot add the ownership inheritance to unit-testing.  I can probably look into using MiniDFS for that, in a follow-up JIRA.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Re: Review Request 20243: HIVE-6891 - Alter rename partition Perm inheritance and general partition/table owner inheritance

Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20243/#review40113
-----------------------------------------------------------


I don't think we will be able to set owner since we are not superuer? But we can set and should set group.

- Brock Noland


On April 11, 2014, 12:49 a.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20243/
> -----------------------------------------------------------
> 
> (Updated April 11, 2014, 12:49 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6891
>     https://issues.apache.org/jira/browse/HIVE-6891
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> This is a follow-up of HIVE-6648.
> 
> Extending the fix to other partition/table operations as well, by refactoring the fixed code in HIVE-6648 into a common FileUtils helper method, and then using it for all table/partition directory creation operation, when "hive.warehouse.subdir.inherit.perms" flag is set.  
> 
> Another part of this change is to add ownership inheritance in this code as well when creating directories.  Ownership was already inherited for data (HIVE-3756), but not at the table/partitioned-table level.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java ad82f62 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java f1c7b7b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 8345d70 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c62e085 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java f731dab 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2559e0e 
> 
> Diff: https://reviews.apache.org/r/20243/diff/
> 
> 
> Testing
> -------
> 
> Extending the unit test TestFolderPermission to handle all the new cases of directory creation (create table, external table, static partition, dynamic partition, rename partition).
> 
> Unfortunately due to the test using the local file system, I cannot add the ownership inheritance to unit-testing.  I can probably look into using MiniDFS for that, in a follow-up JIRA.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Re: Review Request 20243: HIVE-6891 - Alter rename partition Perm inheritance and general partition/table owner inheritance

Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20243/#review40220
-----------------------------------------------------------

Ship it!


+1 pending tests

- Brock Noland


On April 12, 2014, 11:50 p.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20243/
> -----------------------------------------------------------
> 
> (Updated April 12, 2014, 11:50 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6891
>     https://issues.apache.org/jira/browse/HIVE-6891
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> This is a follow-up of HIVE-6648.
> 
> Extending the fix to other partition/table operations as well, by refactoring the fixed code in HIVE-6648 into a common FileUtils helper method, and then using it for all table/partition directory creation operation, when "hive.warehouse.subdir.inherit.perms" flag is set.  
> 
> Another part of this change is to add ownership inheritance in this code as well when creating directories.  Ownership was already inherited for data (HIVE-3756), but not at the table/partitioned-table level.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java ad82f62 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 47e94ea 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java f1c7b7b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 8345d70 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c62e085 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java f731dab 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2559e0e 
> 
> Diff: https://reviews.apache.org/r/20243/diff/
> 
> 
> Testing
> -------
> 
> Extending the unit test TestFolderPermission to handle all the new cases of directory creation (create table, external table, static partition, dynamic partition, rename partition).
> 
> Unfortunately due to the test using the local file system, I cannot add the ownership inheritance to unit-testing.  I can probably look into using MiniDFS for that, in a follow-up JIRA.
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Re: Review Request 20243: HIVE-6891 - Alter rename partition Perm inheritance and general partition/table owner inheritance

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20243/
-----------------------------------------------------------

(Updated April 12, 2014, 11:50 p.m.)


Review request for hive.


Changes
-------

This patch passes the test for both hadoop profiles.  Making the test use the hadoop-shims, to handle listFile non-existent API in hadoop 0.20.


Bugs: HIVE-6891
    https://issues.apache.org/jira/browse/HIVE-6891


Repository: hive-git


Description
-------

This is a follow-up of HIVE-6648.

Extending the fix to other partition/table operations as well, by refactoring the fixed code in HIVE-6648 into a common FileUtils helper method, and then using it for all table/partition directory creation operation, when "hive.warehouse.subdir.inherit.perms" flag is set.  

Another part of this change is to add ownership inheritance in this code as well when creating directories.  Ownership was already inherited for data (HIVE-3756), but not at the table/partitioned-table level.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/common/FileUtils.java ad82f62 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 47e94ea 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java f1c7b7b 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 8345d70 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c62e085 
  metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java f731dab 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2559e0e 

Diff: https://reviews.apache.org/r/20243/diff/


Testing
-------

Extending the unit test TestFolderPermission to handle all the new cases of directory creation (create table, external table, static partition, dynamic partition, rename partition).

Unfortunately due to the test using the local file system, I cannot add the ownership inheritance to unit-testing.  I can probably look into using MiniDFS for that, in a follow-up JIRA.


Thanks,

Szehon Ho


Re: Review Request 20243: HIVE-6891 - Alter rename partition Perm inheritance and general partition/table owner inheritance

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20243/
-----------------------------------------------------------

(Updated April 12, 2014, 7:59 a.m.)


Review request for hive.


Changes
-------

Fixing a unit test, that used Warehouse.mkdir().


Bugs: HIVE-6891
    https://issues.apache.org/jira/browse/HIVE-6891


Repository: hive-git


Description
-------

This is a follow-up of HIVE-6648.

Extending the fix to other partition/table operations as well, by refactoring the fixed code in HIVE-6648 into a common FileUtils helper method, and then using it for all table/partition directory creation operation, when "hive.warehouse.subdir.inherit.perms" flag is set.  

Another part of this change is to add ownership inheritance in this code as well when creating directories.  Ownership was already inherited for data (HIVE-3756), but not at the table/partitioned-table level.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/common/FileUtils.java ad82f62 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 47e94ea 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java f1c7b7b 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 8345d70 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c62e085 
  metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java f731dab 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2559e0e 

Diff: https://reviews.apache.org/r/20243/diff/


Testing
-------

Extending the unit test TestFolderPermission to handle all the new cases of directory creation (create table, external table, static partition, dynamic partition, rename partition).

Unfortunately due to the test using the local file system, I cannot add the ownership inheritance to unit-testing.  I can probably look into using MiniDFS for that, in a follow-up JIRA.


Thanks,

Szehon Ho


Re: Review Request 20243: HIVE-6891 - Alter rename partition Perm inheritance and general partition/table owner inheritance

Posted by Szehon Ho <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20243/
-----------------------------------------------------------

(Updated April 11, 2014, 11:41 p.m.)


Review request for hive.


Changes
-------

Mkdirs will change only the group as discussed, as hive is not superuser.  Tested on a cluster, verified it works as expected if hive is a member of that group.

I will try to refactor the unit test with MiniDFS to verify the group part in a follow-up JIRA, as I cannot create other groups as its running on real local file system. 


Bugs: HIVE-6891
    https://issues.apache.org/jira/browse/HIVE-6891


Repository: hive-git


Description
-------

This is a follow-up of HIVE-6648.

Extending the fix to other partition/table operations as well, by refactoring the fixed code in HIVE-6648 into a common FileUtils helper method, and then using it for all table/partition directory creation operation, when "hive.warehouse.subdir.inherit.perms" flag is set.  

Another part of this change is to add ownership inheritance in this code as well when creating directories.  Ownership was already inherited for data (HIVE-3756), but not at the table/partitioned-table level.


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/common/FileUtils.java ad82f62 
  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java f1c7b7b 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java 8345d70 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java c62e085 
  metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java f731dab 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 2559e0e 

Diff: https://reviews.apache.org/r/20243/diff/


Testing
-------

Extending the unit test TestFolderPermission to handle all the new cases of directory creation (create table, external table, static partition, dynamic partition, rename partition).

Unfortunately due to the test using the local file system, I cannot add the ownership inheritance to unit-testing.  I can probably look into using MiniDFS for that, in a follow-up JIRA.


Thanks,

Szehon Ho