> On 2011-06-06 22:18:08, Carl Steinbach wrote: > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1153 > > <https://reviews.apache.org/r/831/diff/2/?file=20589#file20589line1153> > > > > 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.
Sounds good. > On 2011-06-06 22:18:08, Carl Steinbach wrote: > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1164 > > <https://reviews.apache.org/r/831/diff/2/?file=20589#file20589line1164> > > > > 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. Ah ok thanks; it felt strange to throw a NoSuchObjectException there. > On 2011-06-06 22:18:08, Carl Steinbach wrote: > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 1171 > > <https://reviews.apache.org/r/831/diff/2/?file=20589#file20589line1171> > > > > 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. > > I see your concern, I'll change it to retrieve only tables that are in the metastore. > On 2011-06-06 22:18:08, Carl Steinbach wrote: > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, > > line 818 > > <https://reviews.apache.org/r/831/diff/2/?file=20590#file20590line818> > > > > 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? > > Sounds good. Removing the ordering especially makes sense if we cannot guarantee that we return *every* table the caller requested. > On 2011-06-06 22:18:08, Carl Steinbach wrote: > > trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, > > line 802 > > <https://reviews.apache.org/r/831/diff/2/?file=20590#file20590line802> > > > > It would be good to first check if the DB exists, and throw > > UnknownDBException if it's not found. Will do. As a side note, this check requires another db call, which may slow down the function a little. - Sohan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/831/#review768 ----------------------------------------------------------- On 2011-06-07 01:11:44, Sohan Jain wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/831/ > ----------------------------------------------------------- > > (Updated 2011-06-07 01:11:44) > > > 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 > >
