hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carl Steinbach" <c...@cloudera.com>
Subject Re: Review Request: HIVE-2188: Add a function to retrieve multiple tables on trip to the hive metastore
Date Mon, 06 Jun 2011 22:18:08 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/831/#review768
-----------------------------------------------------------



trunk/metastore/if/hive_metastore.thrift
<https://reviews.apache.org/r/831/#comment1650>

    Please consider changing this to "get_table_objects_by_name"
    
    This should also throw InvalidObjectException and UnknownDBException.
    



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/831/#comment1648>

    Maybe change this to "get_table_objects_by_name" in order to disambiguate from cases where
we're returning only tables names, or applying a filter condition, etc. etc.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/831/#comment1647>

    This should probably be InvalidOperationException instead of NoSuchObjectException.
    
    It might also be good to validate the dbname input parameter at this step, e.g. make sure
it's not null and not an empty string.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/831/#comment1646>

    Failing the entire operation if a single table in the input list is not defined seems
like a bad idea since we're throwing away work that will have to be repeated on the next call.
Furthermore, the exception doesn't contain any information about which table(s) are not defined,
so the client will have to fetch a table list again and use this to construct the list of
input tables for the next get_multi_table() call. In the meantime it's possible that someone
will drop a table in the list, which will invalidate the next call.
    
    I think it would be better to modify the contract to state that if a table on the input
list is not found in the metastore, then the table definition will not be included in the
result. This means that the function will return an empty list if none of the tables in the
input list are found in the metastore.
    



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/831/#comment1645>

    e.toString() actually returns a little more information than e.getMessage().



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/831/#comment1649>

    Change to getTableObjectsByName?



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/831/#comment1643>

    It would be good to first check if the DB exists, and throw UnknownDBException if it's
not found.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
<https://reviews.apache.org/r/831/#comment1644>

    Only some callers will care about having this ordering property satisfied, so instead
of penalizing every caller with this performance hit, maybe it would be better to let the
caller take care of this?
    


- Carl


On 2011-06-06 21:09:54, Sohan Jain wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/831/
> -----------------------------------------------------------
> 
> (Updated 2011-06-06 21:09:54)
> 
> 
> Review request for hive, Paul Yang and Ashutosh Chauhan.
> 
> 
> Summary
> -------
> 
> Created a function "multi_get_table" that retrieves multiple tables on one trip to the
hive metastore, saving round trip time.
> 
> 
> This addresses bug HIVE-2188.
>     https://issues.apache.org/jira/browse/HIVE-2188
> 
> 
> Diffs
> -----
> 
>   trunk/metastore/if/hive_metastore.thrift 1130342 
>   trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1130342

>   trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1130342

>   trunk/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 1130342 
>   trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java 1130342

> 
> Diff: https://reviews.apache.org/r/831/diff
> 
> 
> Testing
> -------
> 
> Added a test case to testMetasore() in TestHiveServer.  Also tested for speed improvements
in a client session.
> 
> 
> Thanks,
> 
> Sohan
> 
>


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