You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Steven Phillips <sp...@maprtech.com> on 2015/06/29 22:42:25 UTC

Review Request 36019: Patch for DRILL-3418

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

Review request for drill.


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


Repository: drill-git


Description
-------

DRILL-3414: Make sure to walk entire expression tree when rewriting filter expression for pruning


Diffs
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteAsBinaryOperators.java 44b9a3a8fbd22744f98b9b4b64c9b7aceae7587a 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteCombineBinaryOperators.java 247ad8f0fa883f6c765d94edac58d1b5e2193ddb 
  exec/java-exec/src/test/java/org/apache/drill/TestCTASPartitionFilter.java 48d7cebb26d2bf08baff39d6232e4829bd98d648 

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


Testing
-------


Thanks,

Steven Phillips


Re: Review Request 36019: Patch for DRILL-3418

Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36019/#review89805
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteAsBinaryOperators.java (line 110)
<https://reviews.apache.org/r/36019/#comment142600>

    You may consider overriding visitLocalRef() in both visitors. The default one in RexVisitorImpl just return null, which seems not right.  On the other hand, I'm also not clear when/where LocalRef would be created in Rex expression. It probably would not cause big issue for partiton pruning if not override. For consisentency, it might be better to do so.
    
      public R visitLocalRef(RexLocalRef localRef) {
        return null;
      }


- Jinfeng Ni


On June 29, 2015, 1:42 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36019/
> -----------------------------------------------------------
> 
> (Updated June 29, 2015, 1:42 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-3418
>     https://issues.apache.org/jira/browse/DRILL-3418
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-3414: Make sure to walk entire expression tree when rewriting filter expression for pruning
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteAsBinaryOperators.java 44b9a3a8fbd22744f98b9b4b64c9b7aceae7587a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteCombineBinaryOperators.java 247ad8f0fa883f6c765d94edac58d1b5e2193ddb 
>   exec/java-exec/src/test/java/org/apache/drill/TestCTASPartitionFilter.java 48d7cebb26d2bf08baff39d6232e4829bd98d648 
> 
> Diff: https://reviews.apache.org/r/36019/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>


Re: Review Request 36019: Patch for DRILL-3418

Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36019/#review89828
-----------------------------------------------------------

Ship it!



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteCombineBinaryOperators.java (line 94)
<https://reviews.apache.org/r/36019/#comment142636>

    Include the type argument here ?


- Aman Sinha


On June 29, 2015, 10:09 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36019/
> -----------------------------------------------------------
> 
> (Updated June 29, 2015, 10:09 p.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-3418
>     https://issues.apache.org/jira/browse/DRILL-3418
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-3414: Make sure to walk entire expression tree when rewriting filter expression for pruning
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteAsBinaryOperators.java 44b9a3a8fbd22744f98b9b4b64c9b7aceae7587a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteCombineBinaryOperators.java 247ad8f0fa883f6c765d94edac58d1b5e2193ddb 
>   exec/java-exec/src/test/java/org/apache/drill/TestCTASPartitionFilter.java 48d7cebb26d2bf08baff39d6232e4829bd98d648 
> 
> Diff: https://reviews.apache.org/r/36019/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>


Re: Review Request 36019: Patch for DRILL-3418

Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36019/#review89830
-----------------------------------------------------------

Ship it!


Ship It!

- Aman Sinha


On June 30, 2015, 12:09 a.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36019/
> -----------------------------------------------------------
> 
> (Updated June 30, 2015, 12:09 a.m.)
> 
> 
> Review request for drill.
> 
> 
> Bugs: DRILL-3418
>     https://issues.apache.org/jira/browse/DRILL-3418
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-3414: Make sure to walk entire expression tree when rewriting filter expression for pruning
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteAsBinaryOperators.java 44b9a3a8fbd22744f98b9b4b64c9b7aceae7587a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteCombineBinaryOperators.java 247ad8f0fa883f6c765d94edac58d1b5e2193ddb 
>   exec/java-exec/src/test/java/org/apache/drill/TestCTASPartitionFilter.java 48d7cebb26d2bf08baff39d6232e4829bd98d648 
> 
> Diff: https://reviews.apache.org/r/36019/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>


Re: Review Request 36019: Patch for DRILL-3418

Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36019/
-----------------------------------------------------------

(Updated June 30, 2015, 12:09 a.m.)


Review request for drill.


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


Repository: drill-git


Description
-------

DRILL-3414: Make sure to walk entire expression tree when rewriting filter expression for pruning


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteAsBinaryOperators.java 44b9a3a8fbd22744f98b9b4b64c9b7aceae7587a 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteCombineBinaryOperators.java 247ad8f0fa883f6c765d94edac58d1b5e2193ddb 
  exec/java-exec/src/test/java/org/apache/drill/TestCTASPartitionFilter.java 48d7cebb26d2bf08baff39d6232e4829bd98d648 

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


Testing
-------


Thanks,

Steven Phillips


Re: Review Request 36019: Patch for DRILL-3418

Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36019/
-----------------------------------------------------------

(Updated June 29, 2015, 10:09 p.m.)


Review request for drill.


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


Repository: drill-git


Description
-------

DRILL-3414: Make sure to walk entire expression tree when rewriting filter expression for pruning


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteAsBinaryOperators.java 44b9a3a8fbd22744f98b9b4b64c9b7aceae7587a 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/partition/RewriteCombineBinaryOperators.java 247ad8f0fa883f6c765d94edac58d1b5e2193ddb 
  exec/java-exec/src/test/java/org/apache/drill/TestCTASPartitionFilter.java 48d7cebb26d2bf08baff39d6232e4829bd98d648 

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


Testing
-------


Thanks,

Steven Phillips