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



itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java
<https://reviews.apache.org/r/25616/#comment92869>

    This line that sets concurrency false can be removed.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java
<https://reviews.apache.org/r/25616/#comment92870>

    It would be good to also verify the input columns being passed here.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java
<https://reviews.apache.org/r/25616/#comment92871>

    A similar test for delete would also be useful, specially for testing the 
input columns being passed.



ql/src/java/org/apache/hadoop/hive/ql/Driver.java
<https://reviews.apache.org/r/25616/#comment92872>

    Can you also change the variable name of tab2cols to indicate that it is 
the table to input column mapping (since we have updateTab2Cols) ? maybe 
selectTab2Cols or inputTab2Cols



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
<https://reviews.apache.org/r/25616/#comment92873>

    For insert-overwrite, both insert and delete was required earlier. 
    This would be 
    
     new PrivRequirement(arr(SQLPrivTypeGrant.INSERT_NOGRANT, 
SQLPrivTypeGrant.DELETE_NOGRANT), IOType.OUTPUT, 
HivePrivObjectActionType.INSERT_OVERWRITE),



ql/src/test/queries/clientpositive/authorization_update.q
<https://reviews.apache.org/r/25616/#comment92874>

    yes, This is something we really need to fix in hive! I think comment 
before dfs,add/delete, compile etc also has same issue. Vikram had taken a stab 
at fixing this, but I believe the fix turned out to be more involved that 
expected.


- Thejas Nair


On Sept. 14, 2014, 4:30 a.m., Alan Gates wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25616/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2014, 4:30 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-7790
>     https://issues.apache.org/jira/browse/HIVE-7790
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Adds update and delete as action and adds checks for authorization during 
> update and delete. Also adds passing of updated columns in case authorizer 
> wishes to check them.
> 
> 
> Diffs
> -----
> 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java
>  53d88b0 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 298f429 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> b2f66e0 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java 
> 3aaa09c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  93df9f4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  093b4fd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  3236341 
>   ql/src/test/queries/clientnegative/authorization_delete_nodeletepriv.q 
> PRE-CREATION 
>   ql/src/test/queries/clientnegative/authorization_update_noupdatepriv.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_delete.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_delete_own_table.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_update.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/authorization_update_own_table.q 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_delete_nodeletepriv.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_update_noupdatepriv.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/authorization_delete.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/authorization_delete_own_table.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/authorization_update.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/authorization_update_own_table.q.out 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25616/diff/
> 
> 
> Testing
> -------
> 
> Added tests, both positive and negative, for update and delete, including 
> ability to update and delete tables created by user.  Also added tests for 
> passing correct update columns.
> 
> 
> Thanks,
> 
> Alan Gates
> 
>

Reply via email to