----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/187/#review76 -----------------------------------------------------------
http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java <https://reviews.apache.org/r/187/#comment114> Here and elsewhere in this file and ObjectStore, why are we throwing RuntimeExceptions instead of MetaExceptions? http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java <https://reviews.apache.org/r/187/#comment115> We should just ignore attempts to create a duplicate grant (of either role or privilege). But if the old one does not have grant option and the new one does, then we should upgrade it in place. (Spoke with Yongqiang; we'll defer this to a followup since it requires adding an update API to the metastore for the WITH GRANT OPTION upgrade.) http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/package.jdo <https://reviews.apache.org/r/187/#comment101> Make this 128. http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/package.jdo <https://reviews.apache.org/r/187/#comment113> When we define column-level privs (independent of partition), will PART_ID be null? That would be bad since a unique index ignores duplicates in the presence of any nulls. Do we have something like a NO_PARTITION_ID reserved value? Another option is to use a single key column TABLE_OR_PART_ID and add an addition boolean IS_PARTITION (true for partition-level, false for table-level). http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/package.jdo <https://reviews.apache.org/r/187/#comment102> Make this 128. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java <https://reviews.apache.org/r/187/#comment103> Don't use printStackTrace http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java <https://reviews.apache.org/r/187/#comment104> should be Class.forName(className, true, JavaUtils.getClassLoader()) http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java <https://reviews.apache.org/r/187/#comment105> should be Class.forName(className, true, JavaUtils.getClassLoader()) http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g <https://reviews.apache.org/r/187/#comment106> should be "revoke role" http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g <https://reviews.apache.org/r/187/#comment107> should be "show role grants" http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HiveAuthenticationProvider.java <https://reviews.apache.org/r/187/#comment108> What is the return value of this method supposed to indicate? Make it void and expect an exception instead. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProvider.java <https://reviews.apache.org/r/187/#comment110> Should be "Hive's pluggable authorization provider interface" http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProvider.java <https://reviews.apache.org/r/187/#comment109> This should take no parameters, and should rely on setConf already having been called. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/Privilege.java <https://reviews.apache.org/r/187/#comment111> Javadoc? http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeRegistry.java <https://reviews.apache.org/r/187/#comment112> Javadoc? - John On 2010-12-20 14:30:53, John Sichi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/187/ > ----------------------------------------------------------- > > (Updated 2010-12-20 14:30:53) > > > Review request for hive. > > > Summary > ------- > > review by JVS > > > This addresses bug HIVE-78. > https://issues.apache.org/jira/browse/HIVE-78 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 1050266 > http://svn.apache.org/repos/asf/hive/trunk/conf/hive-default.xml 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/if/hive_metastore.thrift > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MColumnPrivilege.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MDBPrivilege.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MGlobalPrivilege.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MRole.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MRoleMap.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/org/apache/hadoop/hive/metastore/model/MTablePartitionPrivilege.java > PRE-CREATION > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/model/package.jdo > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/SMBMapJoinOperator.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/AuthorizationException.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/GrantDesc.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/GrantRevokeRoleDDL.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/PrincipalDesc.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeDesc.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/RevokeDesc.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/RoleDDLDesc.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultAuthenticator.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HiveAuthenticationProvider.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveAuthorizationProvider.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProvider.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/Privilege.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeRegistry.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeScope.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/CreateTableAutomaticGrant.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/QTestUtil.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/metadata/TestSemanticAnalyzerHookLoading.java > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/DummyAuthenticator.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_fail_1.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_fail_2.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_fail_3.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_fail_4.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_fail_5.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_fail_6.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_fail_7.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/authorization_part.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/authorization_1.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/authorization_2.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/authorization_3.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/authorization_4.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/input19.q > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/show_indexes_edge_cases.q > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_fail_1.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_fail_2.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_fail_3.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_fail_4.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_fail_5.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_fail_6.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_fail_7.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientnegative/authorization_part.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/alter4.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/authorization_1.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/authorization_2.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/authorization_3.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/authorization_4.q.out > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/bucket_groupby.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/create_default_prop.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/ctas.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/input19.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/merge3.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/query_result_fileformat.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/rcfile_default_format.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/semijoin.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/show_indexes_edge_cases.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/smb_mapjoin9.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats10.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats12.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats13.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats2.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats5.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats6.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats7.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats8.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/stats9.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/str_to_map.q.out > 1050266 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/updateAccessTime.q.out > 1050266 > > Diff: https://reviews.apache.org/r/187/diff > > > Testing > ------- > > > Thanks, > > John > >
