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

Koji Noguchi commented on PIG-5328:
-----------------------------------

Thanks Michael!  +1 on the patch.
Confirmed your testcase for divide-0 fails without your changes and works after 
the change.

Also, I see that {{TestPOCast.java}} is missing {{testBigDecimalToOther}} test 
method which would have been a good place to add the testing for typecasting 
BigDecimal to boolean.  I can create a new jira for that.

I'll commit your patch tomorrow.

> expressionOperator Divide.equalsZero(DataType.BIGDECIMAL) is invalid
> --------------------------------------------------------------------
>
>                 Key: PIG-5328
>                 URL: https://issues.apache.org/jira/browse/PIG-5328
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.16.0, 0.17.0
>         Environment: pig source HEAD as of Jan 2018 ... probably goes all the 
> way back to initial implementation of BigDecimal support
>            Reporter: Michael Howard
>            Assignee: Michael Howard
>            Priority: Major
>         Attachments: patchPig5328-take01.patch
>
>   Original Estimate: 0.25h
>          Time Spent: 2h
>  Remaining Estimate: 0h
>
> Divide.equalsZero(DataType.BIGDECIMAL) is flawed in that it uses an invalid 
> test for == ZERO in the case of BigDecimal. 
>  
> ./physicalLayer/expressionOperators/Divide.java tests the divisor for zero in 
> order to avoid DivideByZero.
> The test is performed using a method equalsZero(...)
> Divide.equalsZero() is given 'protected' access, but I could not find other 
> references ... should be 'private'
> equalsZero() implementation dispatches on dataType to type-specific 
> predicates ... the BigDecimal implementation is incorrect 
> The method BigDecimal.equals(other) is intended to be used for object 
> equality, not numerical equality. (Their justification is that equals() is 
> used in hash-table lookups in java Collections.) BigDecimal numbers are not 
> normalized and scale is an important attribute. Scale is included in 
> BigDecimal.equals(). The values "0" and "0.00" have different scales and are 
> not considered "equals()"
> Comparisons for numeric equality need to be done using compareTo()
> In the special case of comparing to zero, BigDecimal.signum() is the best. 
> The current code is
> {{     case DataType.BIGDECIMAL:}}
> {{            return BigDecimal.ZERO.equals((BigDecimal) a);}}
> needs to be changed to
> {{     case DataType.BIGDECIMAL:}}
> {{            return ((BigDecimal) a).signum() == 0;}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to