You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Venki Korukanti <ve...@gmail.com> on 2015/04/15 07:28:12 UTC
Review Request 33209: DRILL-2341: Type information is lost if list of
fields is specified during create view
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/
-----------------------------------------------------------
Review request for drill and Jinfeng Ni.
Repository: drill-git
Description
-------
Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
Diff: https://reviews.apache.org/r/33209/diff/
Testing
-------
Includes unittests.
Thanks,
Venki Korukanti
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/#review80193
-----------------------------------------------------------
Ship it!
Ship It!
- Jinfeng Ni
On April 15, 2015, 7:50 a.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33209/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 7:50 a.m.)
>
>
> Review request for drill and Jinfeng Ni.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 300d65d
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 325f770
> exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
>
> Diff: https://reviews.apache.org/r/33209/diff/
>
>
> Testing
> -------
>
> Includes unittests.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/
-----------------------------------------------------------
(Updated April 15, 2015, 2:50 p.m.)
Review request for drill and Jinfeng Ni.
Changes
-------
Addressed review comments.
Repository: drill-git
Description
-------
Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 300d65d
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 325f770
exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
Diff: https://reviews.apache.org/r/33209/diff/
Testing
-------
Includes unittests.
Thanks,
Venki Korukanti
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Venki Korukanti <ve...@gmail.com>.
> On April 15, 2015, 6:11 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java, line 114
> > <https://reviews.apache.org/r/33209/diff/1/?file=929615#file929615line114>
> >
> > One question. Seems you call DrillRelOptUtil.createRename(), just to get the rowtype of this new project. Why can't you create a rowType with the desired field name directly?
Let me if this still makes sense after the refactoring.
- Venki
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/#review80165
-----------------------------------------------------------
On April 15, 2015, 2:50 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33209/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:50 p.m.)
>
>
> Review request for drill and Jinfeng Ni.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 300d65d
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 325f770
> exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
>
> Diff: https://reviews.apache.org/r/33209/diff/
>
>
> Testing
> -------
>
> Includes unittests.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/#review80165
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
<https://reviews.apache.org/r/33209/#comment129929>
One question. Seems you call DrillRelOptUtil.createRename(), just to get the rowtype of this new project. Why can't you create a rowType with the desired field name directly?
- Jinfeng Ni
On April 14, 2015, 10:28 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33209/
> -----------------------------------------------------------
>
> (Updated April 14, 2015, 10:28 p.m.)
>
>
> Review request for drill and Jinfeng Ni.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
> exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
>
> Diff: https://reviews.apache.org/r/33209/diff/
>
>
> Testing
> -------
>
> Includes unittests.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Venki Korukanti <ve...@gmail.com>.
> On April 15, 2015, 5:45 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java, line 114
> > <https://reviews.apache.org/r/33209/diff/1/?file=929615#file929615line114>
> >
> > This part of logic is quite similar in ViewHandler and CreateTableHandler. Is it possible that we move the common code to a helper method (check the field count, *, create the rename project, etc), so that both of the Handlers could call this method?
Good idea. Refactored in v2 patch.
- Venki
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/#review80159
-----------------------------------------------------------
On April 15, 2015, 2:50 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33209/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:50 p.m.)
>
>
> Review request for drill and Jinfeng Ni.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 300d65d
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 325f770
> exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
>
> Diff: https://reviews.apache.org/r/33209/diff/
>
>
> Testing
> -------
>
> Includes unittests.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/#review80159
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java
<https://reviews.apache.org/r/33209/#comment129925>
This part of logic is quite similar in ViewHandler and CreateTableHandler. Is it possible that we move the common code to a helper method (check the field count, *, create the rename project, etc), so that both of the Handlers could call this method?
- Jinfeng Ni
On April 14, 2015, 10:28 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33209/
> -----------------------------------------------------------
>
> (Updated April 14, 2015, 10:28 p.m.)
>
>
> Review request for drill and Jinfeng Ni.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
> exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
>
> Diff: https://reviews.apache.org/r/33209/diff/
>
>
> Testing
> -------
>
> Includes unittests.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Venki Korukanti <ve...@gmail.com>.
> On April 15, 2015, 5:35 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java, line 96
> > <https://reviews.apache.org/r/33209/diff/1/?file=929614#file929614line96>
> >
> > can you create a jira (and not it here) which tracks removing this and is dependent on Jinfeng's calcite rebase?
Logged DRILL-2797
- Venki
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/#review80158
-----------------------------------------------------------
On April 15, 2015, 2:50 p.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33209/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 2:50 p.m.)
>
>
> Review request for drill and Jinfeng Ni.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 300d65d
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java 325f770
> exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
>
> Diff: https://reviews.apache.org/r/33209/diff/
>
>
> Testing
> -------
>
> Includes unittests.
>
>
> Thanks,
>
> Venki Korukanti
>
>
Re: Review Request 33209: DRILL-2341: Type information is lost if
list of fields is specified during create view
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33209/#review80158
-----------------------------------------------------------
Ship it!
One small note. Otherwise, lgtm
exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java
<https://reviews.apache.org/r/33209/#comment129924>
can you create a jira (and not it here) which tracks removing this and is dependent on Jinfeng's calcite rebase?
- Jacques Nadeau
On April 15, 2015, 5:28 a.m., Venki Korukanti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33209/
> -----------------------------------------------------------
>
> (Updated April 15, 2015, 5:28 a.m.)
>
>
> Review request for drill and Jinfeng Ni.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Create a cast to rename the view definition query row type to view field list specified in CREATE VIEW statement.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java cacf26b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 4347249
> exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java 578eace
>
> Diff: https://reviews.apache.org/r/33209/diff/
>
>
> Testing
> -------
>
> Includes unittests.
>
>
> Thanks,
>
> Venki Korukanti
>
>