You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Sean Hsuan-Yi Chu <hs...@usc.edu> on 2015/05/21 08:12:52 UTC

Review Request 34530: DRILL-3130: Add DrillUnionRule

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

Review request for drill, Aman Sinha and Jinfeng Ni.


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


Repository: drill-git


Description
-------

Add DrillPushProjectPastUnionRule to push project below union


Diffs
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java f7cfbf4 
  exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 5f98d90 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectFiltertPushDownOverUnionAll.tsv PRE-CREATION 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithProject.tsv PRE-CREATION 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithoutProject.tsv PRE-CREATION 

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


Testing
-------

Unit,, TPCH, (waiting for functional)


Thanks,

Sean Hsuan-Yi Chu


Re: Review Request 34530: DRILL-3130: Add DrillUnionRule

Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34530/
-----------------------------------------------------------

(Updated May 22, 2015, 10:33 p.m.)


Review request for drill, Aman Sinha and Jinfeng Ni.


Changes
-------

a new patch


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


Repository: drill-git


Description
-------

Add DrillPushProjectPastUnionRule to push project below union


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java f7cfbf4 
  exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 5f98d90 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectDownOverUnionAllImplicitCasting.tsv PRE-CREATION 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectFiltertPushDownOverUnionAll.tsv PRE-CREATION 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithProject.tsv PRE-CREATION 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithoutProject.tsv PRE-CREATION 
  exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectWithExpressionPushDownOverUnionAll.tsv PRE-CREATION 

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


Testing
-------

Unit,, TPCH, (waiting for functional)


Thanks,

Sean Hsuan-Yi Chu


Re: Review Request 34530: DRILL-3130: Add DrillUnionRule

Posted by Sean Hsuan-Yi Chu <hs...@usc.edu>.

> On May 22, 2015, 7:01 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java, line 48
> > <https://reviews.apache.org/r/34530/diff/1/?file=966108#file966108line48>
> >
> >     With this new rule, what will happen if we are projecting an expression after a UNION-ALL ? for example: 
> >      SELECT 2 * y FROM 
> >        (SELECT n_nationkey y FROM nation 
> >         UNION ALL 
> >         SELECT n_regionkey y FROM nation
> >        )

I updated a new patch to push this project, along with its expression, downward

Also, I also add a test-case where the inputs of union-all have different types.
Thus, before this push down patch, implicit casting happens in union-all, but now it could happen in the project which is pushed down.


For example,

select 2 * n_nationkey as col from 
(select n_nationkey, n_name, n_comment from cp.`tpch/nation.parquet`
union all select columns[0], columns[1], columns[2] from dfs.`csv file`)

will give:

00-00    Screen
00-01      Project(col=[$0])
00-02        UnionAll(all=[true])
00-04          Project(n_nationkey=[*(2, $0)])
00-06            Scan(groupscan=[ParquetGroupScan [entries=[ReadEntryWithPath [path=classpath:/tpch/nation.parquet]], selectionRoot=/tpch/nation.parquet, numFiles=1, columns=[`n_nationkey`]]])
00-03          Project(EXPR$0=[*(2, ITEM($0, 0))])
00-05            Scan(groupscan=[EasyGroupScan [selectionRoot=/Users/hyichu/Documents/SourceTree/incubator-drill/exec/java-exec/target/test-classes/store/text/data/nations.csv, numFiles=1, columns=[`columns`[0]], files=[file:/Users/hyichu/Documents/SourceTree/incubator-drill/exec/java-exec/target/test-classes/store/text/data/nations.csv]]])

Instead of being in union-all, an implict casting happens in the 
00-03          Project(EXPR$0=[*(2, ITEM($0, 0))])


- Sean Hsuan-Yi


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


On May 21, 2015, 6:12 a.m., Sean Hsuan-Yi Chu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34530/
> -----------------------------------------------------------
> 
> (Updated May 21, 2015, 6:12 a.m.)
> 
> 
> Review request for drill, Aman Sinha and Jinfeng Ni.
> 
> 
> Bugs: DRILL-3130
>     https://issues.apache.org/jira/browse/DRILL-3130
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Add DrillPushProjectPastUnionRule to push project below union
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java f7cfbf4 
>   exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 5f98d90 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectFiltertPushDownOverUnionAll.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithProject.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithoutProject.tsv PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34530/diff/
> 
> 
> Testing
> -------
> 
> Unit,, TPCH, (waiting for functional)
> 
> 
> Thanks,
> 
> Sean Hsuan-Yi Chu
> 
>


Re: Review Request 34530: DRILL-3130: Add DrillUnionRule

Posted by Aman Sinha <as...@maprtech.com>.

> On May 22, 2015, 7:01 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java, line 48
> > <https://reviews.apache.org/r/34530/diff/1/?file=966108#file966108line48>
> >
> >     With this new rule, what will happen if we are projecting an expression after a UNION-ALL ? for example: 
> >      SELECT 2 * y FROM 
> >        (SELECT n_nationkey y FROM nation 
> >         UNION ALL 
> >         SELECT n_regionkey y FROM nation
> >        )
> 
> Sean Hsuan-Yi Chu wrote:
>     I updated a new patch to push this project, along with its expression, downward
>     
>     Also, I also add a test-case where the inputs of union-all have different types.
>     Thus, before this push down patch, implicit casting happens in union-all, but now it could happen in the project which is pushed down.
>     
>     
>     
>     For example,
>     
>     select 2 * n_nationkey as col from 
>     (select n_nationkey, n_name, n_comment from cp.`tpch/nation.parquet`
>     union all select columns[0], columns[1], columns[2] from dfs.`csv file`)
>     
>     will give:
>     
>     00-00    Screen
>     00-01      Project(col=[$0])
>     00-02        UnionAll(all=[true])
>     00-04          Project(n_nationkey=[*(2, $0)])
>     00-06            Scan(groupscan=[ParquetGroupScan [entries=[ReadEntryWithPath [path=classpath:/tpch/nation.parquet]], selectionRoot=/tpch/nation.parquet, numFiles=1, columns=[`n_nationkey`]]])
>     00-03          Project(EXPR$0=[*(2, ITEM($0, 0))])
>     00-05            Scan(groupscan=[EasyGroupScan [selectionRoot=/Users/hyichu/Documents/SourceTree/incubator-drill/exec/java-exec/target/test-classes/store/text/data/nations.csv, numFiles=1, columns=[`columns`[0]], files=[file:/Users/hyichu/Documents/SourceTree/incubator-drill/exec/java-exec/target/test-classes/store/text/data/nations.csv]]])
>     
>     Instead of being in union-all, an implict casting happens in the 
>     00-03          Project(EXPR$0=[*(2, ITEM($0, 0))])

I think we would have to be more careful about the projection push-down for expressions.  Can you run some tests with other expressions such as CAST, LEN (of a varchar).  For instance if the two sides of the union have VARCHAR(10), VARCHAR(20) and we are doing a LEN() after the UNION, what is the result ?


- Aman


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


On May 22, 2015, 10:33 p.m., Sean Hsuan-Yi Chu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34530/
> -----------------------------------------------------------
> 
> (Updated May 22, 2015, 10:33 p.m.)
> 
> 
> Review request for drill, Aman Sinha and Jinfeng Ni.
> 
> 
> Bugs: DRILL-3130
>     https://issues.apache.org/jira/browse/DRILL-3130
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Add DrillPushProjectPastUnionRule to push project below union
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java f7cfbf4 
>   exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 5f98d90 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectDownOverUnionAllImplicitCasting.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectFiltertPushDownOverUnionAll.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithProject.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithoutProject.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectWithExpressionPushDownOverUnionAll.tsv PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34530/diff/
> 
> 
> Testing
> -------
> 
> Unit,, TPCH, (waiting for functional)
> 
> 
> Thanks,
> 
> Sean Hsuan-Yi Chu
> 
>


Re: Review Request 34530: DRILL-3130: Add DrillUnionRule

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



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java
<https://reviews.apache.org/r/34530/#comment136420>

    With this new rule, what will happen if we are projecting an expression after a UNION-ALL ? for example: 
     SELECT 2 * y FROM 
       (SELECT n_nationkey y FROM nation 
        UNION ALL 
        SELECT n_regionkey y FROM nation
       )


- Aman Sinha


On May 21, 2015, 6:12 a.m., Sean Hsuan-Yi Chu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34530/
> -----------------------------------------------------------
> 
> (Updated May 21, 2015, 6:12 a.m.)
> 
> 
> Review request for drill, Aman Sinha and Jinfeng Ni.
> 
> 
> Bugs: DRILL-3130
>     https://issues.apache.org/jira/browse/DRILL-3130
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Add DrillPushProjectPastUnionRule to push project below union
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjectPastUnionRule.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java f7cfbf4 
>   exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 5f98d90 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectFiltertPushDownOverUnionAll.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithProject.tsv PRE-CREATION 
>   exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testProjectPushDownOverUnionAllWithoutProject.tsv PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34530/diff/
> 
> 
> Testing
> -------
> 
> Unit,, TPCH, (waiting for functional)
> 
> 
> Thanks,
> 
> Sean Hsuan-Yi Chu
> 
>