----------------------------------------------------------- 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 > >