Mailing-List: contact dev-help@drill.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@drill.apache.org
Content-Type: multipart/alternative;
 boundary="===============3478139819294637497=="
MIME-Version: 1.0
Subject: Re: Review Request 32886: DRILL-2639: Planner bug -
 RelOptPlanner.CannotPlanExceptio
From: "Sean Hsuan-Yi Chu" <hsuanyi@usc.edu>
To: "Aman Sinha" <asinha@maprtech.com>
Cc: "Sean Hsuan-Yi Chu" <hsuanyi@usc.edu>,
 "drill" <drill-dev@incubator.apache.org>
Date: Thu, 09 Apr 2015 17:17:49 -0000
Message-ID: <20150409171749.1488.85603@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Sean Hsuan-Yi Chu" <noreply@reviews.apache.org>
References: <20150409063804.1489.58901@reviews.apache.org>
In-Reply-To: <20150409063804.1489.58901@reviews.apache.org>
Reply-To: "Sean Hsuan-Yi Chu" <hsuanyi@usc.edu>

--===============3478139819294637497==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


--===============3478139819294637497==--