hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Sichi" <jsi...@fb.com>
Subject Re: Review Request: HIVE-78 patch 10
Date Mon, 20 Dec 2010 22:30:57 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message