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
