FAQ
Author: ehans
Date: Mon Feb 17 20:50:25 2014
New Revision: 1569111

URL: http://svn.apache.org/r1569111
Log:
HIVE-6399: bug in high-precision Decimal128 multiply (Eric Hanson, reviewed by Jitendra Pandey)

Modified:
     hive/trunk/common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java
     hive/trunk/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java

Modified: hive/trunk/common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java
URL: http://svn.apache.org/viewvc/hive/trunk/common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java?rev=1569111&r1=1569110&r2=1569111&view=diff
==============================================================================
--- hive/trunk/common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java (original)
+++ hive/trunk/common/src/java/org/apache/hadoop/hive/common/type/Decimal128.java Mon Feb 17 20:50:25 2014
@@ -1053,6 +1053,12 @@ public final class Decimal128 extends Nu
    }

    /**
+ * As of 2/11/2014 this has a known bug in multiplication. See
+ * TestDecimal128.testKnownPriorErrors(). Keeping this source
+ * code available since eventually it is better to fix this.
+ * The fix employed now is to replace this code with code that
+ * uses Java BigDecimal multiply.
+ *
     * Performs multiplication, changing the scale of this object to the specified
     * value.
     *
@@ -1061,7 +1067,7 @@ public final class Decimal128 extends Nu
     * @param newScale
     * scale of the result. must be 0 or positive.
     */
- public void multiplyDestructive(Decimal128 right, short newScale) {
+ public void multiplyDestructiveNativeDecimal128(Decimal128 right, short newScale) {
      if (this.signum == 0 || right.signum == 0) {
        this.zeroClear();
        this.scale = newScale;
@@ -1102,6 +1108,31 @@ public final class Decimal128 extends Nu
    }

    /**
+ * Performs multiplication, changing the scale of this object to the specified
+ * value.
+ *
+ * @param right
+ * right operand. this object is not modified.
+ * @param newScale
+ * scale of the result. must be 0 or positive.
+ */
+ public void multiplyDestructive(Decimal128 right, short newScale) {
+ HiveDecimal rightHD = HiveDecimal.create(right.toBigDecimal());
+ HiveDecimal thisHD = HiveDecimal.create(this.toBigDecimal());
+ HiveDecimal result = thisHD.multiply(rightHD);
+
+ /* If the result is null, throw an exception. This can be caught
+ * by calling code in the vectorized code path and made to yield
+ * a SQL NULL value.
+ */
+ if (result == null) {
+ throw new ArithmeticException("null multiply result");
+ }
+ this.update(result.bigDecimalValue().toPlainString(), newScale);
+ this.unscaledValue.throwIfExceedsTenToThirtyEight();
+ }
+
+ /**
     * Performs division and puts the result into the given object. Both operands
     * are first scaled up/down to the specified scale, then this method performs
     * the operation. This method is static and not destructive (except the result

Modified: hive/trunk/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java
URL: http://svn.apache.org/viewvc/hive/trunk/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java?rev=1569111&r1=1569110&r2=1569111&view=diff
==============================================================================
--- hive/trunk/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java (original)
+++ hive/trunk/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java Mon Feb 17 20:50:25 2014
@@ -51,39 +51,19 @@ public class TestDecimal128 {
    }

    @Test
- public void testCalculateTenThirtyEight() {
+ public void testCalculateTenThirtySeven() {

- // find 10^38
+ // find 10^37
      Decimal128 ten = new Decimal128(10, (short) 0);
      Decimal128 val = new Decimal128(1, (short) 0);
- for (int i = 0; i < 38; ++i) {
+ for (int i = 0; i < 37; ++i) {
        val.multiplyDestructive(ten, (short) 0);
      }

      // verify it
      String s = val.toFormalString();
- assertEquals("100000000000000000000000000000000000000", s);
+ assertEquals("10000000000000000000000000000000000000", s);
      boolean overflow = false;
-
- // show that it is is an overflow for precision 38
- try {
- val.checkPrecisionOverflow(38);
- } catch (Exception e) {
- overflow = true;
- }
- assertTrue(overflow);
-
- // subtract one
- val.subtractDestructive(one, (short) 0);
- overflow = false;
-
- // show that it does not overflow for precision 38
- try {
- val.checkPrecisionOverflow(38);
- } catch (Exception e) {
- overflow = true;
- }
- assertFalse(overflow);
    }

    @Test
@@ -377,6 +357,13 @@ public class TestDecimal128 {
      long a = 213474114411690L;
      long b = 5062120663L;
      verifyMultiplyDivideInverse(a, b);
+
+ // Regression test for defect reported in HIVE-6399
+ String a2 = "-605044214913338382"; // 18 digits
+ String b2 = "55269579109718297360"; // 20 digits
+
+ // -33440539101030154945490585226577271520 is expected result
+ verifyHighPrecisionMultiplySingle(a2, b2);
    }

    // Test a set of random adds at high precision.
@@ -415,7 +402,8 @@ public class TestDecimal128 {
      String res2 = bdR.toPlainString();

      // Compare the results
- assertEquals(res1, res2);
+ String message = "For operation " + a.toFormalString() + " + " + b.toFormalString();
+ assertEquals(message, res2, res1);
    }

    // Test a set of random subtracts at high precision.
@@ -454,7 +442,8 @@ public class TestDecimal128 {
      String res2 = bdR.toPlainString();

      // Compare the results
- assertEquals(res1, res2);
+ String message = "For operation " + a.toFormalString() + " - " + b.toFormalString();
+ assertEquals(message, res2, res1);
    }

    // Test a set of random multiplications at high precision.
@@ -490,7 +479,7 @@ public class TestDecimal128 {

      String res1 = r.toFormalString();

- // Now do the add with Java BigDecimal
+ // Now do the operation with Java BigDecimal
      BigDecimal bdA = new BigDecimal(sA);
      BigDecimal bdB = new BigDecimal(sB);
      BigDecimal bdR = bdA.multiply(bdB);
@@ -498,9 +487,52 @@ public class TestDecimal128 {
      String res2 = bdR.toPlainString();

      // Compare the results
- assertEquals(res1, res2);
+ String message = "For operation " + a.toFormalString() + " * " + b.toFormalString();
+ assertEquals(message, res2, res1);
    }

+ // Test a single, high-precision multiply of random inputs.
+ // Arguments must be integers with optional - sign, represented as strings.
+ // Arguments must have 1 to 37 digits and the number of total digits
+ // must be <= 38.
+ private void verifyHighPrecisionMultiplySingle(String argA, String argB) {
+
+ Decimal128 a, b, r;
+ String sA, sB;
+
+ // verify number of digits is <= 38 and each number has 1 or more digits
+ int aDigits = argA.length();
+ aDigits -= argA.charAt(0) == '-' ? 1 : 0;
+ int bDigits = argB.length();
+ bDigits -= argB.charAt(0) == '-' ? 1 : 0;
+ assertTrue(aDigits + bDigits <= 38 && aDigits > 0 && bDigits > 0);
+
+ a = new Decimal128();
+ sA = argA;
+ a.update(sA, (short) 0);
+ b = new Decimal128();
+ sB = argB;
+ b.update(sB, (short) 0);
+
+ r = new Decimal128();
+ r.addDestructive(a, (short) 0);
+ r.multiplyDestructive(b, (short) 0);
+
+ String res1 = r.toFormalString();
+
+ // Now do the operation with Java BigDecimal
+ BigDecimal bdA = new BigDecimal(sA);
+ BigDecimal bdB = new BigDecimal(sB);
+ BigDecimal bdR = bdA.multiply(bdB);
+
+ String res2 = bdR.toPlainString();
+
+ // Compare the results
+ String message = "For operation " + a.toFormalString() + " * " + b.toFormalString();
+ assertEquals(message, res2, res1);
+ }
+
+
    // Test a set of random divisions at high precision.
    @Test
    public void testHighPrecisionDecimal128Divide() {
@@ -558,7 +590,8 @@ public class TestDecimal128 {
      String res2 = bdR.toPlainString();

      // Compare the results
- assertEquals(res1, res2);
+ String message = "For operation " + a.toFormalString() + " / " + b.toFormalString();
+ assertEquals(message, res2, res1);
    }

    /* Return a random number with length digits, as a string. Results may be

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 1 of 1 | next ›
Discussion Overview
groupcommits @
categorieshive, hadoop
postedFeb 17, '14 at 8:50p
activeFeb 17, '14 at 8:50p
posts1
users1
websitehive.apache.org

1 user in discussion

Ehans: 1 post

People

Translate

site design / logo © 2021 Grokbase