[ 
https://issues.apache.org/jira/browse/HIVE-5520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13793250#comment-13793250
 ] 

Ashutosh Chauhan commented on HIVE-5520:
----------------------------------------

All the ctors which were throwing exceptions earlier {{ throw new 
NumberFormatException("Assignment would result in truncation"); }} are now 
silently returning null in those cases, resulting in almost all mathematical 
functions like divide (), setScale(), remainder() etc to return null in error 
conditions instead of throwing exception. Is that desirable ? e.g,, I think if 
divide () fails on BigDecimal that should throw exception instead of returning 
null.

> Use factory methods to instantiate HiveDecimal instead of constructors
> ----------------------------------------------------------------------
>
>                 Key: HIVE-5520
>                 URL: https://issues.apache.org/jira/browse/HIVE-5520
>             Project: Hive
>          Issue Type: Improvement
>          Components: Types
>    Affects Versions: 0.11.0
>            Reporter: Xuefu Zhang
>            Assignee: Xuefu Zhang
>             Fix For: 0.13.0
>
>         Attachments: HIVE-5520.1.patch, HIVE-5520.patch
>
>
> Currently HiveDecimal class provided a bunch of constructors that  
> unfortunately also throws a runtime exception. For example,
> {code}
>  public HiveDecimal(BigInteger unscaled, int scale) {
>     bd = this.normalize(new BigDecimal(unscaled, scale), MAX_PRECISION, 
> false);
>     if (bd == null) {
>      throw new NumberFormatException("Assignment would result in truncation");
>    }
> {code}
> As a result, it's hard for the caller to detect error occurrences and the 
> error handling is also complicated. In many cases, the error handling is 
> omitted or missed. For instance,
> {code}
>          HiveDecimalWritable result = new 
> HiveDecimalWritable(HiveDecimal.ZERO);
>         try {
>           result.set(aggregation.sum.divide(new 
> HiveDecimal(aggregation.count)));
>         } catch (NumberFormatException e) {
>           result = null;
>         }
> {code} 
> Throwing runtime exception while expecting caller to catch seems 
> anti-pattern. In the case of constructor, factory class or methods seem more 
> appropriate. With such a change, the apis are cleaner, and the error handling 
> is simplified.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to