Uploaded image for project: 'Pig'
  1. Pig
  2. PIG-5328

expressionOperator Divide.equalsZero(DataType.BIGDECIMAL) is invalid

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.16.0, 0.17.0
    • Fix Version/s: 0.18.0
    • Component/s: impl
    • Labels:
      None
    • Environment:

      pig source HEAD as of Jan 2018 ... probably goes all the way back to initial implementation of BigDecimal support

    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      the division operator checks for a zero denominator to prevent a divide-by-zero exception. Unlike other numeric types, BigDecimal has multiple representations of zero with different scales ... 0, 0.0, 0.00, etc. Previous versions of pig were were not properly detecting zero values with non-zero scales when performing denominator check prior to division.
      Show
      the division operator checks for a zero denominator to prevent a divide-by-zero exception. Unlike other numeric types, BigDecimal has multiple representations of zero with different scales ... 0, 0.0, 0.00, etc. Previous versions of pig were were not properly detecting zero values with non-zero scales when performing denominator check prior to division.

      Description

      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;

       

        Attachments

        1. patchPig5328-take01.patch
          4 kB
          Michael Howard

          Activity

            People

            • Assignee:
              michaelthoward Michael Howard
              Reporter:
              michaelthoward Michael Howard
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 0.25h Original Estimate - 0.25h
                0.25h
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 2h
                2h