> On Sept. 16, 2014, 6:42 a.m., Thejas Nair wrote: > > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 741 > > <https://reviews.apache.org/r/25616/diff/2/?file=690379#file690379line741> > > > > should we skip it from ReadEntity if none of the columns are being used > > ? Though, that case is not going to be common. > > > > eg a query like 'update table set j=null;' should not require select > > privileges on the table, as there is no columns in where clause or value > > expression. > > > > Note that this is a change that we can also make in future without > > breaking users. (making a change in future to require fewer privileges will > > not break users). Ie, it does not have to be addressed in this patch. > > Alan Gates wrote: > I don't think that makes any sense. If I have delete permissions but not > select permissions I can delete all rows from a table but not some rows? > That definitely violates the law of least astonishment.
You need select privileges only for use of columns in the value expression (in update) and where clause (in update/delete). In case of an authorization implementation that supports column level access control on table, this is the behavior you get. You will be able to delete all rows even if you don't have select privileges on the columns. For example, you could have an application that sets some fields with PII to null. It would not need select privileges. I am fine with current approach, as we have the ability to change it in future, based on user feedback. - Thejas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25616/#review53485 ----------------------------------------------------------- On Sept. 16, 2014, 7:37 p.m., Alan Gates wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25616/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2014, 7:37 p.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/ColumnAccessInfo.java a4df8b4 > > 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 > >
