> On May 21, 2014, 10:22 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java, > > line 51 > > <https://reviews.apache.org/r/21749/diff/1/?file=585933#file585933line51> > > > > Better name : HivePrivType?
This is an object type and not a privilege type. > On May 21, 2014, 10:22 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 763 > > <https://reviews.apache.org/r/21749/diff/1/?file=585932#file585932line763> > > > > What other writeType is legal here? This should throw unsupported > > writeType instead I think. For purposes of authorization other write types are not considered. I have renamed DEFAULT to OTHER . I will clarify in updated comment. > On May 21, 2014, 10:22 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java, > > line 55 > > <https://reviews.apache.org/r/21749/diff/1/?file=585933#file585933line55> > > > > Better name : HivePrivActionType ? Object in there sounds redundant ? > > This type is applicable only within an object. There is a higher level HiveOperation for the high level operation (eg DROP_TABLE, QUERY, LOAD ..), but each object in that operation might have different actions happening on it. I think the word object in it gives better indication of where its used/applicable. Do you want me to shorten it to HiveObjectActionType ? > On May 21, 2014, 10:22 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java, > > line 56 > > <https://reviews.apache.org/r/21749/diff/1/?file=585933#file585933line56> > > > > Why do we need DEFAULT here? Its good to document what actions it > > covers. Else, I think UNKNOWN is better name. Changed to OTHER > On May 21, 2014, 10:22 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java, > > line 40 > > <https://reviews.apache.org/r/21749/diff/1/?file=585934#file585934line40> > > > > Better name : RequiredPrivs? This is a private class that has privilege requirement and the conditions under which the privilege requirement is applicable. Operation2Privilege.getRequiredPrivs returns a RequiredPrivileges object after applying the conditions in it. How about PrivRequirementCondition ? - Thejas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21749/#review43659 ----------------------------------------------------------- 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 > >
