> On Jan. 13, 2017, 2:22 p.m., Barna Zsombor Klara wrote: > > 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.
Thanks a lot for the review! > On Jan. 13, 2017, 2:22 p.m., Barna Zsombor Klara wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java, > > line 288 > > <https://reviews.apache.org/r/55498/diff/1/?file=1604621#file1604621line288> > > > > 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. Good point! :) I extracted the pattern into a constant in HiveMetaStore and used it in the test. > On Jan. 13, 2017, 2:22 p.m., Barna Zsombor Klara wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java, > > line 267 > > <https://reviews.apache.org/r/55498/diff/1/?file=1604621#file1604621line267> > > > > 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. I tried it and it was quite easy, so added the verification of the returned values. - Marta ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55498/#review161524 ----------------------------------------------------------- On Jan. 13, 2017, 3:26 p.m., Marta Kuczora wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55498/ > ----------------------------------------------------------- > > (Updated Jan. 13, 2017, 3:26 p.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 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 121b825 > > 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 > >
