----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21749/#review43659 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/Driver.java <https://reviews.apache.org/r/21749/#comment77941> To keep Driver file size in control, shall we move this to AuthorizationUtils class ? ql/src/java/org/apache/hadoop/hive/ql/Driver.java <https://reviews.apache.org/r/21749/#comment77943> This kind of null check may result in DEFAULT being returned which will be undesirable. I think instead of that we should make sure that writeType can never be null here and don't do null check here. ql/src/java/org/apache/hadoop/hive/ql/Driver.java <https://reviews.apache.org/r/21749/#comment77944> What other writeType is legal here? This should throw unsupported writeType instead I think. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java <https://reviews.apache.org/r/21749/#comment77948> Better name : HivePrivType? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java <https://reviews.apache.org/r/21749/#comment77947> Better name : HivePrivActionType ? Object in there sounds redundant ? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java <https://reviews.apache.org/r/21749/#comment77942> Why do we need DEFAULT here? Its good to document what actions it covers. Else, I think UNKNOWN is better name. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java <https://reviews.apache.org/r/21749/#comment77949> Better name : RequiredPrivs? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java <https://reviews.apache.org/r/21749/#comment77950> In what case, actionType could be null ? Good to document here. - Ashutosh Chauhan On May 21, 2014, 1:49 a.m., Thejas Nair wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21749/ > ----------------------------------------------------------- > > (Updated May 21, 2014, 1:49 a.m.) > > > Review request for hive, Ashutosh Chauhan and Thejas Nair. > > > Bugs: HIVE-7061 > https://issues.apache.org/jira/browse/HIVE-7061 > > > Repository: hive-git > > > Description > ------- > > See bug > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 9040d9b > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java > a3a689d > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java > b0a804c > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java > 229c063 > ql/src/test/queries/clientnegative/authorization_insertoverwrite_nodel.q > PRE-CREATION > ql/src/test/queries/clientpositive/authorization_insert.q PRE-CREATION > ql/src/test/results/clientnegative/authorization_insert_noinspriv.q.out > ee8d49e > > ql/src/test/results/clientnegative/authorization_insertoverwrite_nodel.q.out > PRE-CREATION > ql/src/test/results/clientpositive/authorization_insert.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/21749/diff/ > > > Testing > ------- > > Test included. > > > Thanks, > > Thejas Nair > >
