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