> On jan. 8, 2019, 10:04 de, Peter Vary wrote: > > Thanks for the patch! > > Two nits below. > > Also a bit concerned about the size calculation - seems ok, but it would be > > good to have a few test case which validates the contentsummary > > calculations (when every path is cached/only few patch is cached/no path is > > cached), so we can be sure that further changes will not break the > > functionality. > > > > What do you think? > > > > Peter > > David Mollitor wrote: > Thank you Peter for the review. This functionality is unit tested > already. Do you have suggestions for additional unit tests? > > > > https://github.com/apache/hive/blob/ae008b79b5d52ed6a38875b73025a505725828eb/ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java
I missed the ones which are testing the full getInputSummary method, and found only the ones (testGetInputSummaryPool, testGetInputSummaryPoolAndFailure) using the getInputSummaryWithPool. Seeing those tests I feel much better. Maybe it is an edge case, but it might be good to add a test where we call the getInputSummary twice. First with one set of path (p1, p2) and then again with another set of path (p1, p2, p3, p4) so we can check that the merge of the cached results and the newly fetched ones is working. - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69683/#review211758 ----------------------------------------------------------- On jan. 14, 2019, 3:36 du, David Mollitor wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69683/ > ----------------------------------------------------------- > > (Updated jan. 14, 2019, 3:36 du) > > > Review request for hive. > > > Repository: hive-git > > > Description > ------- > > Improve performance of method getInputSummary by changing data structures and > allowing multiple threads to do calculations. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 2ff9ad3 > > > Diff: https://reviews.apache.org/r/69683/diff/2/ > > > Testing > ------- > > Unit > > > Thanks, > > David Mollitor > >
