----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72526/#review220811 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 1711 (patched) <https://reviews.apache.org/r/72526/#comment309516> Let's set it to false by default until the cost-based config is in place. If you identify some tests that are affected by the optimization, move them to a new file and set it to true specifically for that file to have some test coverage. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java Lines 54 (patched) <https://reviews.apache.org/r/72526/#comment309518> private static final ? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java Lines 68 (patched) <https://reviews.apache.org/r/72526/#comment309517> Do you need this? Even if it is not a Project, optimization should still be applicable: You just need to create input refs for _n_ fields, where _n_ is the number of fields in the root node. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java Lines 183 (patched) <https://reviews.apache.org/r/72526/#comment309519> It seems you use `projectExpr.getIndex`. I think we should use the position in the `List<> projectExpressions` rather than the index. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java Lines 230 (patched) <https://reviews.apache.org/r/72526/#comment309520> Use _RexUtil.composeConjunction_ so the AND is flattened. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java Lines 269 (patched) <https://reviews.apache.org/r/72526/#comment309521> You can just use a Pair here. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java Lines 325 (patched) <https://reviews.apache.org/r/72526/#comment309522> Do you need to override this method? ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Lines 2396 (patched) <https://reviews.apache.org/r/72526/#comment309523> TODO: Encapsulate this within a rule. ql/src/test/queries/clientpositive/cardinality_preserving_join_opt_q11.q Lines 1 (patched) <https://reviews.apache.org/r/72526/#comment309524> These tests should be moved to Perf constraints driver before check-in. If there are constraints missing in the driver e.g. unique constraints, please add them. - Jesús Camacho Rodríguez On May 18, 2020, 6:31 p.m., Krisztian Kasa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72526/ > ----------------------------------------------------------- > > (Updated May 18, 2020, 6:31 p.m.) > > > Review request for hive and Jesús Camacho Rodríguez. > > > Bugs: HIVE-23493 > https://issues.apache.org/jira/browse/HIVE-23493 > > > Repository: hive-git > > > Description > ------- > > Rewrite plan to join back tables with many projected columns joined multiple > times > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java f5ad3a882b > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java > 19ce3ea223 > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 32ad4c1539 > ql/src/test/queries/clientpositive/cardinality_preserving_join_opt.q > PRE-CREATION > ql/src/test/queries/clientpositive/cardinality_preserving_join_opt_q11.q > PRE-CREATION > ql/src/test/queries/clientpositive/cardinality_preserving_join_opt_q4.q > PRE-CREATION > ql/src/test/queries/clientpositive/cardinality_preserving_join_opt_q74.q > PRE-CREATION > > ql/src/test/results/clientpositive/llap/cardinality_preserving_join_opt.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/llap/cardinality_preserving_join_opt_q11.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/llap/cardinality_preserving_join_opt_q4.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/llap/cardinality_preserving_join_opt_q74.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/72526/diff/1/ > > > Testing > ------- > > mvn test -Dtest.output.overwrite -DskipSparkTests > -Dtest=TestMiniLlapLocalCliDriver > -Dqfile=cardinality_preserving_join_opt.q,cardinality_preserving_join_opt_q4.q,cardinality_preserving_join_opt_q11.q,cardinality_preserving_join_opt_q74.q > -pl itests/qtest -Pitests > > > Thanks, > > Krisztian Kasa > >
