[
https://issues.apache.org/jira/browse/HIVE-5520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13793435#comment-13793435
]
Xuefu Zhang commented on HIVE-5520:
-----------------------------------
Hive current error handling is to set NULL value in case of errors, such as
divide-by-zero, data underflow or overflow, etc. Instead of letting almost
every caller to do something like this:
{code}
try {
HiveDecimal d = new HiveDecimal(...);
catch (NumberFormatException e) {
return null;
}
{code}
I think it's simpler and cleaner just to do this:
{code}
return HiveDecimal.create(...);
{code}
Another reason to returning null is that HiveDecimal doesn't have a NULL
instance, yet null value is expected in input/output for cases such error or
miss value. Relying caller to catch a runtime exception and propagate null
seems requiring due diligence. I saw many cases this is omitted or missed.
Since Hive puts null for error conditions, it seems natural to return null from
multiple() or divide() because that's what the callers will do anyway.
Let me know if you have further thoughts. Thanks.
> 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)
