drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Hsuan-Yi Chu" <hsua...@usc.edu>
Subject Re: Review Request 32886: DRILL-2639: Planner bug - RelOptPlanner.CannotPlanExceptio
Date Thu, 09 Apr 2015 17:17:49 GMT


> On April 9, 2015, 6:38 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java,
line 506
> > <https://reviews.apache.org/r/32886/diff/1/?file=916507#file916507line506>
> >
> >     I don't think you need a new utility method.. you can get the major type from
Types.getMajorTypeFromName() and supply it the sqlType.getName() and you can get the minor
type from major type.

Two reasons:
1. Types.getMajorTypeFromName() doese not cover all SqlTypeName. For example, SqlTypeName.SYMBOL
would not be caught in any "case" in the switch logic. In my new method, I am sure every case
is covered.
2. Using "String" type as switch logic could sometimes be risky. Especially here, when we
can have the option to use 'enum', it seems easier and safer to use enum, as opposed to casting
to String and switch-case.


- Sean Hsuan-Yi


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


On April 6, 2015, 4:45 p.m., Sean Hsuan-Yi Chu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32886/
> -----------------------------------------------------------
> 
> (Updated April 6, 2015, 4:45 p.m.)
> 
> 
> Review request for drill and Aman Sinha.
> 
> 
> Bugs: DRILL-2639
>     https://issues.apache.org/jira/browse/DRILL-2639
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> At planning for Union-All, block only the cases where implicit casting cannot resolve
an output type
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java
aba6022 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillUnionRelBase.java
932aa76 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java
796f0f7 
>   exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java fee1d6a 
> 
> Diff: https://reviews.apache.org/r/32886/diff/
> 
> 
> Testing
> -------
> 
> QA, unit
> 
> 
> Thanks,
> 
> Sean Hsuan-Yi Chu
> 
>


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