You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Yuliya Feldman <yu...@yahoo.com> on 2015/02/13 06:29:13 UTC

Review Request 30965: Follow up on DRILL-133 (LocalExchange) to save CPU cycles on hash generation when using in HashToLocalExchange

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30965/
-----------------------------------------------------------

Review request for drill, Jacques Nadeau, Steven Phillips, and Venki Korukanti.


Bugs: DRILL-2209
    https://issues.apache.org/jira/browse/DRILL-2209


Repository: drill-git


Description
-------

Insert Project operator to add new column "EXPRHASH" with hash expression for fields that are used for HashToRandomExchange
Remove Project operator after HashRandomExchange (or Demux) since it will create problems to fields ordering in HashJoin.

Tight this to MuxExchange - so if MuxExchange is enabled, Project is inserted.


Diffs
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashToRandomExchangePrel.java 372c75d 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java PRE-CREATION 

Diff: https://reviews.apache.org/r/30965/diff/


Testing
-------

Need to add Unit Tests. tested live, run Functional and TPCH tests


Thanks,

Yuliya Feldman


Re: Review Request 30965: Follow up on DRILL-133 (LocalExchange) to save CPU cycles on hash generation when using in HashToLocalExchange

Posted by Chris Westin <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30965/#review73231
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java
<https://reviews.apache.org/r/30965/#comment119494>

    Won't prel.getCluster().getRexBuilder() be the same for every pass of the loop body? Do it once before and reuse.
    
    Same for child.getRowType().getFieldList(). (And child.getRowType() could be reused up above as well).
    
    In this line, you call field.getFieldId() twice, when you could call it once before this line, and then use that value twice.



exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java
<https://reviews.apache.org/r/30965/#comment119495>

    More use of prel.getCluster().getRexBuilder().



exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java
<https://reviews.apache.org/r/30965/#comment119496>

    More use of prel.getCluster().getRexBuilder().



exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java
<https://reviews.apache.org/r/30965/#comment119497>

    prel.getCluster() here and down below.


- Chris Westin


On Feb. 12, 2015, 9:29 p.m., Yuliya Feldman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30965/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2015, 9:29 p.m.)
> 
> 
> Review request for drill, Jacques Nadeau, Steven Phillips, and Venki Korukanti.
> 
> 
> Bugs: DRILL-2209
>     https://issues.apache.org/jira/browse/DRILL-2209
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Insert Project operator to add new column "EXPRHASH" with hash expression for fields that are used for HashToRandomExchange
> Remove Project operator after HashRandomExchange (or Demux) since it will create problems to fields ordering in HashJoin.
> 
> Tight this to MuxExchange - so if MuxExchange is enabled, Project is inserted.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashToRandomExchangePrel.java 372c75d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30965/diff/
> 
> 
> Testing
> -------
> 
> Need to add Unit Tests. tested live, run Functional and TPCH tests
> 
> 
> Thanks,
> 
> Yuliya Feldman
> 
>


Re: Review Request 30965: Follow up on DRILL-133 (LocalExchange) to save CPU cycles on hash generation when using in HashToLocalExchange

Posted by Yuliya Feldman <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30965/
-----------------------------------------------------------

(Updated Feb. 20, 2015, 5:46 p.m.)


Review request for drill, Jacques Nadeau, Steven Phillips, and Venki Korukanti.


Changes
-------

Updated based of first code review round


Bugs: DRILL-2209
    https://issues.apache.org/jira/browse/DRILL-2209


Repository: drill-git


Description
-------

Insert Project operator to add new column "EXPRHASH" with hash expression for fields that are used for HashToRandomExchange
Remove Project operator after HashRandomExchange (or Demux) since it will create problems to fields ordering in HashJoin.

Tight this to MuxExchange - so if MuxExchange is enabled, Project is inserted.


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/HashToRandomExchangePrel.java 372c75d 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/visitor/InsertLocalExchangeVisitor.java PRE-CREATION 

Diff: https://reviews.apache.org/r/30965/diff/


Testing
-------

Need to add Unit Tests. tested live, run Functional and TPCH tests


Thanks,

Yuliya Feldman