> On May 18, 2020, 11:45 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java > > Lines 183 (patched) > > <https://reviews.apache.org/r/72526/diff/1/?file=2232534#file2232534line183> > > > > It seems you use `projectExpr.getIndex`. I think we should use the > > position in the `List<> projectExpressions` rather than the index.
These indices are not always the same: ``` create table if not exists customer ( c_customer_sk bigint, c_customer_id int, c_first_name string, c_last_name string ); create table store_sales ( ss_customer_sk int, ss_customer_id int, ss_quantity int, ss_list_price float ); select ss_customer_sk, ss_customer_id, c_last_name, ss_list_price, c_customer_sk, c_customer_id, c_first_name, ss_quantity from store_sales ss join customer c on ss.ss_customer_sk = c.c_customer_sk and ss_customer_id = c_customer_id; ``` In this case the third element in the project list is the `c_last_name` column from the `customer` table which is the 8th element in the rowtype of the input of the Project operator. `((RexSlot)rootProject.getProjects().get(2)).getIndex()` is `8` > On May 18, 2020, 11:45 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java > > Lines 269 (patched) > > <https://reviews.apache.org/r/72526/diff/1/?file=2232534#file2232534line269> > > > > You can just use a Pair here. That was my first intention but after a while I was totally lost what was `left` and `right` means in this context. Then I decided to introduced this `ProjectMapping` class with more describing field names. > On May 18, 2020, 11:45 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveCardinalityPreservingJoinOptimization.java > > Lines 325 (patched) > > <https://reviews.apache.org/r/72526/diff/1/?file=2232534#file2232534line325> > > > > Do you need to override this method? I only add collecting the `HiveTableScan` instances in order to copy them when the Join operators created. - Krisztian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72526/#review220811 ----------------------------------------------------------- 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 > >