drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sudheesh Katkam" <skat...@maprtech.com>
Subject Re: Review Request 35629: DRILL-3316: Ensure different SQLHandlers should go through the same planning logics for the same SELECT statement.
Date Mon, 22 Jun 2015 17:47:52 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message