You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Jinfeng Ni <jn...@maprtech.com> on 2015/06/19 00:11:54 UTC

Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

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

Review request for drill and Venki Korukanti.


Repository: drill-git


Description
-------

n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.


Diffs
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 

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


Testing
-------

Unit test.
Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).


Thanks,

Jinfeng Ni


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

Posted by Sudheesh Katkam <sk...@maprtech.com>.

> On June 18, 2015, 10:44 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java, line 157
> > <https://reviews.apache.org/r/35629/diff/1/?file=987598#file987598line157>
> >
> >     Request: as part of this patch, since this method is doing more than logging, can you rename it to something more appropriate?
> 
> Jinfeng Ni wrote:
>     Agree. 
>     
>     If it's fine for you, I'll open a sepearte JIRA to make the change, since it's a slightly different issue from the one that this JIRA is trying to address.
> 
> Sudheesh Katkam wrote:
>     IMO the change is too small for a JIRA, and this patch does refactoring. Anyway, I filed [DRILL-3332](https://issues.apache.org/jira/browse/DRILL-3332) for this.
> 
> Jinfeng Ni wrote:
>     IMO, the change is more appropriate in DRILL-3319, which you filed days ago, since they both handled the refactoring of logging issue in the code. Mixed different things together would make it a bit harder for people to see why the code is changed that way, especially months later when they checked the descrpition of JIRA.

I was gonna make the change as part of that JIRA but I did not know the appropriate name. I agree that a patch should match the JIRA description. On the other hand, I hope we can get to resolve these small issues.


- Sudheesh


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


On June 20, 2015, 12:49 a.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 12:49 a.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> -------
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

Posted by Jinfeng Ni <jn...@maprtech.com>.

> On June 18, 2015, 3:44 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java, line 157
> > <https://reviews.apache.org/r/35629/diff/1/?file=987598#file987598line157>
> >
> >     Request: as part of this patch, since this method is doing more than logging, can you rename it to something more appropriate?

Agree. 

If it's fine for you, I'll open a sepearte JIRA to make the change, since it's a slightly different issue from the one that this JIRA is trying to address.


- Jinfeng


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


On June 19, 2015, 5:49 p.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> -----------------------------------------------------------
> 
> (Updated June 19, 2015, 5:49 p.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> -------
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

Posted by Sudheesh Katkam <sk...@maprtech.com>.

> On June 18, 2015, 10:44 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java, line 157
> > <https://reviews.apache.org/r/35629/diff/1/?file=987598#file987598line157>
> >
> >     Request: as part of this patch, since this method is doing more than logging, can you rename it to something more appropriate?
> 
> Jinfeng Ni wrote:
>     Agree. 
>     
>     If it's fine for you, I'll open a sepearte JIRA to make the change, since it's a slightly different issue from the one that this JIRA is trying to address.

IMO the change is too small for a JIRA, and this patch does refactoring. Anyway, I filed [DRILL-3332](https://issues.apache.org/jira/browse/DRILL-3332) for this.


- Sudheesh


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


On June 20, 2015, 12:49 a.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 12:49 a.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> -------
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

Posted by Jinfeng Ni <jn...@maprtech.com>.

> On June 18, 2015, 3:44 p.m., Sudheesh Katkam wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java, line 157
> > <https://reviews.apache.org/r/35629/diff/1/?file=987598#file987598line157>
> >
> >     Request: as part of this patch, since this method is doing more than logging, can you rename it to something more appropriate?
> 
> Jinfeng Ni wrote:
>     Agree. 
>     
>     If it's fine for you, I'll open a sepearte JIRA to make the change, since it's a slightly different issue from the one that this JIRA is trying to address.
> 
> Sudheesh Katkam wrote:
>     IMO the change is too small for a JIRA, and this patch does refactoring. Anyway, I filed [DRILL-3332](https://issues.apache.org/jira/browse/DRILL-3332) for this.

IMO, the change is more appropriate in DRILL-3319, which you filed days ago, since they both handled the refactoring of logging issue in the code. Mixed different things together would make it a bit harder for people to see why the code is changed that way, especially months later when they checked the descrpition of JIRA.


- Jinfeng


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


On June 19, 2015, 5:49 p.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> -----------------------------------------------------------
> 
> (Updated June 19, 2015, 5:49 p.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> -------
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

Posted by Sudheesh Katkam <sk...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35629/#review88449
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java (line 135)
<https://reviews.apache.org/r/35629/#comment140980>

    Request: as part of this patch, since this method is doing more than logging, can you rename it to something more appropriate?


- Sudheesh Katkam


On June 18, 2015, 10:11 p.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> -----------------------------------------------------------
> 
> (Updated June 18, 2015, 10:11 p.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> -------
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

Posted by Jinfeng Ni <jn...@maprtech.com>.

> On June 19, 2015, 6:39 p.m., Venki Korukanti wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java, line 114
> > <https://reviews.apache.org/r/35629/diff/2/?file=988707#file988707line114>
> >
> >     Do we need the "queryRowType" argument here?

CTAS is a bit special. In order to support "partition by", planner will insert a Project with an additional field under Writer. That logic happens in physical planning. That's why we pass queryRowType in convertToPrel(), in stead of convertToDrel().


> On June 19, 2015, 6:39 p.m., Venki Korukanti wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java, line 48
> > <https://reviews.apache.org/r/35629/diff/2/?file=988711#file988711line48>
> >
> >     Looks like we don't need the planner reference here anymore?

Right. I'll remove this reference.


- Jinfeng


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


On June 19, 2015, 5:49 p.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> -----------------------------------------------------------
> 
> (Updated June 19, 2015, 5:49 p.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> -------
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35629/#review88612
-----------------------------------------------------------

Ship it!


LGTM


exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java (line 113)
<https://reviews.apache.org/r/35629/#comment141206>

    Do we need the "queryRowType" argument here?



exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java (line 48)
<https://reviews.apache.org/r/35629/#comment141207>

    Looks like we don't need the planner reference here anymore?


- Venki Korukanti


On June 19, 2015, 5:49 p.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35629/
> -----------------------------------------------------------
> 
> (Updated June 19, 2015, 5:49 p.m.)
> 
> 
> Review request for drill and Venki Korukanti.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 
> 
> Diff: https://reviews.apache.org/r/35629/diff/
> 
> 
> Testing
> -------
> 
> Unit test.
> Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.

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

(Updated June 19, 2015, 5:49 p.m.)


Review request for drill and Venki Korukanti.


Changes
-------

Revise code based on review comments.


Repository: drill-git


Description
-------

n this JIRA, we are going to refactor part of SqlHandler code in query planning component, such that the same SELECT statement will go through the same planning logic, whether it use DefaultSqlHandler, or CreateTableHandler, or CreateViewHandler, or ExplainHandler.


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java 2866b8c 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java 5e685c8 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java 5924c7e 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java 3edcdb2 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java 0a3393e 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlCreateView.java 57cfde9 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/SqlDropView.java 473dbcb 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DatabaseMetaDataGetColumnsTest.java 5e59e8c 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestInformationSchemaColumns.java 3f820f5 

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


Testing
-------

Unit test.
Pre-commit test. (Found cases where CTAS have different behaviour from regular SELECT statement).


Thanks,

Jinfeng Ni