----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59468/#review176086 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/CountDistinctRewriteProc.java Lines 337 (patched) <https://reviews.apache.org/r/59468/#comment249410> Comment : All aggregations for this GBy run in PARTIAL2 mode. ql/src/java/org/apache/hadoop/hive/ql/optimizer/CountDistinctRewriteProc.java Lines 372 (patched) <https://reviews.apache.org/r/59468/#comment249411> Although this is semantically correct, to achieve data reduction we need to use a constant key since we are now doing aggregations across keys in this GBy. ql/src/test/queries/clientpositive/count_dist_rewrite.q Lines 59-61 (patched) <https://reviews.apache.org/r/59468/#comment249414> Optimization will not fire for these cases. No advantage for adding this variant. Lets remove them. Sorry I suggested them earlier. ql/src/test/results/clientpositive/groupby_sort_11.q.out Lines 28-29 (original), 28-30 (patched) <https://reviews.apache.org/r/59468/#comment249415> In this case original plan was better, since table T1 is bucketed and sorted, all values for keys are together in Mapper, so there is no need to distinct computation. Note the absence of distinct index in rGBy of original plan. This transformation was done by GroupbyOptimizer. We should check if count(DISTINCT KEY._col0:0._col0) is present in rGby too in new optimizer and fire it only if its present. ql/src/test/results/clientpositive/groupby_sort_8.q.out Line 40 (original), 40-41 (patched) <https://reviews.apache.org/r/59468/#comment249418> Previous plan was better because of reason described earlier. ql/src/test/results/clientpositive/nullgroup4.q.out Line 41 (original), 64 (patched) <https://reviews.apache.org/r/59468/#comment249419> Mode: partials? Expected? ql/src/test/results/clientpositive/udf_count.q.out Lines 77 (patched) <https://reviews.apache.org/r/59468/#comment249420> No mGby2 in plan. Probably removed by ReduceSinkDedup since keys are same for 2 Gbys. As I suggested earlier second Gby should have constant key and then it should show up here. - Ashutosh Chauhan On May 25, 2017, 4:03 a.m., pengcheng xiong wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59468/ > ----------------------------------------------------------- > > (Updated May 25, 2017, 4:03 a.m.) > > > Review request for hive, Ashutosh Chauhan and Gopal V. > > > Repository: hive-git > > > Description > ------- > > HIVE-16654 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 2dfc8b6f89 > itests/src/test/resources/testconfiguration.properties 47a13c93b9 > ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java 8b04cd44fa > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/CountDistinctRewriteProc.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/optimizer/Optimizer.java 7dace9076f > ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java 38a9ef2af1 > ql/src/test/queries/clientpositive/count_dist_rewrite.q PRE-CREATION > ql/src/test/results/clientpositive/groupby_sort_11.q.out 2b3bf4a07a > ql/src/test/results/clientpositive/groupby_sort_8.q.out 4faa0757cc > ql/src/test/results/clientpositive/llap/count_dist_rewrite.q.out > PRE-CREATION > ql/src/test/results/clientpositive/nullgroup4.q.out e5a8eeee14 > ql/src/test/results/clientpositive/perf/query16.q.out cf90c0c162 > ql/src/test/results/clientpositive/perf/query28.q.out 78129cf68b > ql/src/test/results/clientpositive/perf/query94.q.out 836b16bf9f > ql/src/test/results/clientpositive/perf/query95.q.out fa94d0842b > ql/src/test/results/clientpositive/udf_count.q.out f60ad0485e > ql/src/test/results/clientpositive/vector_empty_where.q.out b2dec6d7f6 > > > Diff: https://reviews.apache.org/r/59468/diff/2/ > > > Testing > ------- > > > Thanks, > > pengcheng xiong > >
