----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55498/#review161524 -----------------------------------------------------------
Thanks for the patch Marta, +1. And thank you for taking the time to write a unit test for this instead of a qtest. If you can, please take a look at my comments below, but in the current state it is already fine. itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java (line 267) <https://reviews.apache.org/r/55498/#comment232822> Not sure if it can be done easily but could we somehow check the returned values as well? Right now we only know that the query returned the correct nr of records, whether those are the expected values is unclear. itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java (line 288) <https://reviews.apache.org/r/55498/#comment232817> Can we make the original exception message in HiveMetaStore into a public string pattern and use it here to construct the error message? This way if someone edits the message the test should keep working. - Barna Zsombor Klara On Jan. 13, 2017, 11:58 a.m., Marta Kuczora wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55498/ > ----------------------------------------------------------- > > (Updated Jan. 13, 2017, 11:58 a.m.) > > > Review request for hive and Chaoyu Tang. > > > Bugs: HIVE-15538 > https://issues.apache.org/jira/browse/HIVE-15538 > > > Repository: hive-git > > > Description > ------- > > Added unit test for testing HIVE-13884 with more complex queries and > hive.metastore.limit.partition.request enabled. > It covers cases when the query predicates can be pushed down and the number > of partitions can be retrieved via directSQL. > It also covers cases when the number of partitions cannot be retrieved via > directSQL, so it falls back to ORM. > > > Diffs > ----- > > data/files/max_partition_test_input.txt PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/55498/diff/ > > > Testing > ------- > > The patch contains only a new unit test. Ran the test multiple times > successfully. > > > Thanks, > > Marta Kuczora > >
