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



common/src/java/org/apache/hadoop/hive/common/FileUtils.java
<https://reviews.apache.org/r/18168/#comment64865>

    Better named as getPathOrParentThatExists()



common/src/java/org/apache/hadoop/hive/common/FileUtils.java
<https://reviews.apache.org/r/18168/#comment64864>

    Lot of this code is also present in HdfsAuthorizationProvider class, in 
authorize() method. Its worth checking if there can be code sharing here, not 
just for purpose of avoiding code duplication, but also to make sure there is 
consistent behavior in these two scenarios. I think it will make sense here to 
mimic HdfsAuthProvider here.



common/src/java/org/apache/hadoop/hive/common/FileUtils.java
<https://reviews.apache.org/r/18168/#comment64868>

    Perhaps the better test is path.getFileSystem() instanceof LocalFileSystem ?



ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
<https://reviews.apache.org/r/18168/#comment64869>

    Seems like new util method in FileUtils can be used here.



ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java
<https://reviews.apache.org/r/18168/#comment64870>

    Should this be Insert Priv, instead of Create ?


- Ashutosh Chauhan


On Feb. 17, 2014, 4:11 a.m., Thejas Nair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18168/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2014, 4:11 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-5958
>     https://issues.apache.org/jira/browse/HIVE-5958
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Statement such as create table, alter table that specify an path uri should 
> be allowed under the new authorization scheme only if URI(Path) specified has 
> permissions including read/write and ownership of the file/dir and its 
> children.
> Also, fix issue of database not getting set as output for create-table.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java c1f8842 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 83d5bfc 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java 1111c9a 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 0493302 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 0b7c128 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 1f539ef 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java 
> a22a15f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 77388dd 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java 93c89de 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/SessionStateConfigUserAuthenticator.java
>  812105c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  fae6844 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/RequiredPrivileges.java
>  10a582b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java
>  4a9149f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java
>  40461f7 
>   ql/src/test/queries/clientnegative/authorization_addpartition.q 64d8a3d 
>   ql/src/test/queries/clientnegative/authorization_droppartition.q 45ed99b 
>   ql/src/test/queries/clientnegative/authorization_uri_add_partition.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_alterpart_loc.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_altertab_setloc.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_create_table1.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_create_table_ext.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_createdb.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_index.q PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_insert.q PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_insert_local.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_uri_load_data.q 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_addpartition.q.out f4d3b4f 
>   ql/src/test/results/clientnegative/authorization_createview.q.out cb81b83 
>   ql/src/test/results/clientnegative/authorization_ctas.q.out 1070468 
>   ql/src/test/results/clientnegative/authorization_droppartition.q.out 
> 7de553b 
>   ql/src/test/results/clientnegative/authorization_fail_1.q.out ab1abe2 
>   ql/src/test/results/clientnegative/authorization_fail_2.q.out 2c03b65 
>   ql/src/test/results/clientnegative/authorization_fail_3.q.out bfba08a 
>   ql/src/test/results/clientnegative/authorization_fail_4.q.out 34ad4ef 
>   ql/src/test/results/clientnegative/authorization_fail_5.q.out a0289fb 
>   ql/src/test/results/clientnegative/authorization_fail_6.q.out 47f8bd1 
>   ql/src/test/results/clientnegative/authorization_fail_7.q.out a9bf0cc 
>   ql/src/test/results/clientnegative/authorization_grant_table_allpriv.q.out 
> 0e17c94 
>   ql/src/test/results/clientnegative/authorization_grant_table_fail1.q.out 
> 0c83849 
>   
> ql/src/test/results/clientnegative/authorization_grant_table_fail_nogrant.q.out
>  129b5fa 
>   ql/src/test/results/clientnegative/authorization_insert_noinspriv.q.out 
> 6d510f1 
>   ql/src/test/results/clientnegative/authorization_insert_noselectpriv.q.out 
> 5b9b93a 
>   ql/src/test/results/clientnegative/authorization_invalid_priv_v1.q.out 
> 10d1ca8 
>   ql/src/test/results/clientnegative/authorization_invalid_priv_v2.q.out 
> 62aa8da 
>   
> ql/src/test/results/clientnegative/authorization_not_owner_alter_tab_rename.q.out
>  e41702a 
>   
> ql/src/test/results/clientnegative/authorization_not_owner_alter_tab_serdeprop.q.out
>  e41702a 
>   ql/src/test/results/clientnegative/authorization_not_owner_drop_tab.q.out 
> b456aca 
>   ql/src/test/results/clientnegative/authorization_not_owner_drop_view.q.out 
> 2433846 
>   ql/src/test/results/clientnegative/authorization_part.q.out 31dfda9 
>   
> ql/src/test/results/clientnegative/authorization_priv_current_role_neg.q.out 
> f932a3d 
>   ql/src/test/results/clientnegative/authorization_revoke_table_fail1.q.out 
> 0f4c966 
>   ql/src/test/results/clientnegative/authorization_revoke_table_fail2.q.out 
> c671c8a 
>   ql/src/test/results/clientnegative/authorization_select.q.out 1070468 
>   ql/src/test/results/clientnegative/authorization_select_view.q.out e70a79c 
>   ql/src/test/results/clientnegative/authorization_truncate.q.out c188831 
>   ql/src/test/results/clientnegative/authorization_uri_add_partition.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_alterpart_loc.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_altertab_setloc.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_create_table1.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_create_table_ext.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_createdb.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_index.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_insert.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_insert_local.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_load_data.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/exim_22_export_authfail.q.out 1339bbc 
>   ql/src/test/results/clientnegative/exim_23_import_exist_authfail.q.out 
> 22eaac7 
>   ql/src/test/results/clientnegative/exim_24_import_part_authfail.q.out 
> 6eee71e 
>   ql/src/test/results/clientnegative/exim_25_import_nonexist_authfail.q.out 
> fb4224c 
>   ql/src/test/results/clientnegative/load_exist_part_authfail.q.out fbbdd1c 
>   ql/src/test/results/clientnegative/load_nonpart_authfail.q.out 1c364a5 
>   ql/src/test/results/clientnegative/load_part_authfail.q.out afc0aa4 
>   
> ql/src/test/results/clientpositive/alter_rename_partition_authorization.q.out 
> 8a528a1 
>   ql/src/test/results/clientpositive/authorization_1_sql_std.q.out a219478 
>   ql/src/test/results/clientpositive/authorization_2.q.out e21d5f5 
>   ql/src/test/results/clientpositive/authorization_6.q.out bb5ed95 
>   ql/src/test/results/clientpositive/authorization_7.q.out 240a1cc 
>   ql/src/test/results/clientpositive/authorization_8.q.out 4eef13b 
>   ql/src/test/results/clientpositive/authorization_9.q.out ed6cb08 
>   ql/src/test/results/clientpositive/authorization_admin_almighty1.q.out 
> 6fc4897 
>   
> ql/src/test/results/clientpositive/authorization_create_table_owner_privs.q.out
>  b1bce1c 
>   ql/src/test/results/clientpositive/authorization_grant_table_priv.q.out 
> 1e5c031 
>   ql/src/test/results/clientpositive/authorization_owner_actions.q.out 
> 92b8c62 
>   ql/src/test/results/clientpositive/authorization_revoke_table_priv.q.out 
> ae7e716 
>   ql/src/test/results/clientpositive/authorization_view_sqlstd.q.out 3bbb015 
>   ql/src/test/results/clientpositive/exim_21_export_authsuccess.q.out 5b9b81c 
>   ql/src/test/results/clientpositive/exim_22_import_exist_authsuccess.q.out 
> 6746a44 
>   ql/src/test/results/clientpositive/exim_23_import_part_authsuccess.q.out 
> 4e0dfb0 
>   
> ql/src/test/results/clientpositive/exim_24_import_nonexist_authsuccess.q.out 
> 70e9385 
>   ql/src/test/results/clientpositive/index_auth.q.out 2973eb3 
>   ql/src/test/results/clientpositive/load_exist_part_authsuccess.q.out 
> f674f2f 
>   ql/src/test/results/clientpositive/load_nonpart_authsuccess.q.out ca96d95 
>   ql/src/test/results/clientpositive/load_part_authsuccess.q.out 560c582 
> 
> Diff: https://reviews.apache.org/r/18168/diff/
> 
> 
> Testing
> -------
> 
> new tests
> 
> 
> Thanks,
> 
> Thejas Nair
> 
>

Reply via email to