> On June 20, 2017, 12:17 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveAggregate.java > > Lines 137-139 (patched) > > <https://reviews.apache.org/r/59984/diff/1/?file=1747770#file1747770line137> > > > > Seems like this if branch is unnecessary.
I don't think so, most of the calcite code/ hive rules implement RelShuttle interface. So accept should reroute accordingly > On June 20, 2017, 12:17 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveIntersect.java > > Lines 47-49 (patched) > > <https://reviews.apache.org/r/59984/diff/1/?file=1747771#file1747771line47> > > > > Seems like this if branch is unnecessary. same as above > On June 20, 2017, 12:17 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveUnion.java > > Lines 46-48 (patched) > > <https://reviews.apache.org/r/59984/diff/1/?file=1747772#file1747772line46> > > > > Seems like this if branch is unnecessary. same as above > On June 20, 2017, 12:17 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java > > Lines 3088 (patched) > > <https://reviews.apache.org/r/59984/diff/1/?file=1747773#file1747773line3109> > > > > Might be better to initialize this variable to true. Since, value being > > false is known and limited set. I think this works better for readability. e.g. Aggregate looks for grouping sets and if it finds one sets the flag to true and return, otherwise it traveses all its children. Changing the intial flag to true will entail negating this logic and will anyway keep all the code/cases. - Vineet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59984/#review178248 ----------------------------------------------------------- On June 11, 2017, 7:17 p.m., Vineet Garg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59984/ > ----------------------------------------------------------- > > (Updated June 11, 2017, 7:17 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-16838 > https://issues.apache.org/jira/browse/HIVE-16838 > > > Repository: hive-git > > > Description > ------- > > This patch improves plans for subqueries which have not equal corelated > predicates. > Currently to retrieve all possible correlated predicates inner table is > joined with outer query. This is un-necessary in most of the cases (exception > is if subquery has an aggregate). > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveAggregate.java > 63bbdaccfb > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveIntersect.java > 19e1e026f4 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveUnion.java > 7cfb007a9d > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java > 4c99932759 > ql/src/test/queries/clientpositive/subquery_in.q 4ba170a706 > ql/src/test/results/clientpositive/constprog_partitioner.q.out 8c7f9d3f29 > ql/src/test/results/clientpositive/llap/explainuser_1.q.out 8b04bc9261 > ql/src/test/results/clientpositive/llap/subquery_exists.q.out 3004e36c9d > ql/src/test/results/clientpositive/llap/subquery_in.q.out 1f9c9e4474 > ql/src/test/results/clientpositive/llap/subquery_multi.q.out 29516eff82 > ql/src/test/results/clientpositive/llap/subquery_notin.q.out b4af91579b > ql/src/test/results/clientpositive/llap/subquery_scalar.q.out b78df8b9f5 > ql/src/test/results/clientpositive/llap/subquery_select.q.out 202980e975 > ql/src/test/results/clientpositive/llap/subquery_views.q.out 1a21a02a30 > ql/src/test/results/clientpositive/llap/vector_mapjoin_reduce.q.out > d3586e0db2 > ql/src/test/results/clientpositive/perf/query16.q.out a7f93f9ec2 > ql/src/test/results/clientpositive/perf/query94.q.out c5fc9e7f00 > ql/src/test/results/clientpositive/spark/constprog_partitioner.q.out > 3467215d63 > ql/src/test/results/clientpositive/spark/subquery_exists.q.out 8768b45166 > ql/src/test/results/clientpositive/spark/subquery_in.q.out 80a350656d > ql/src/test/results/clientpositive/spark/vector_mapjoin_reduce.q.out > 2f2609f03e > ql/src/test/results/clientpositive/subquery_exists.q.out cfc76520ce > ql/src/test/results/clientpositive/subquery_exists_having.q.out 2c41ff6c33 > ql/src/test/results/clientpositive/subquery_in_having.q.out c4569ba035 > ql/src/test/results/clientpositive/subquery_notexists.q.out 039df03819 > ql/src/test/results/clientpositive/subquery_notexists_having.q.out > fda801d387 > ql/src/test/results/clientpositive/subquery_notin_having.q.out 462dda5e14 > ql/src/test/results/clientpositive/subquery_unqualcolumnrefs.q.out > 03eb4b6ba4 > ql/src/test/results/clientpositive/vector_mapjoin_reduce.q.out 82bef24230 > > > Diff: https://reviews.apache.org/r/59984/diff/1/ > > > Testing > ------- > > * Added new tests > * Pre-commit testing > > > Thanks, > > Vineet Garg > >
