> 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
> 
>

Reply via email to