diff --git a/common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java b/common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java index fb3c346..82fef7f 100644 --- a/common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java +++ b/common/src/java/org/apache/hadoop/hive/common/type/UnsignedInt128.java @@ -54,6 +54,20 @@ public static final UnsignedInt128 TEN_TO_THIRTYEIGHT = new UnsignedInt128(0, 0x98a2240, 0x5a86c47a, 0x4b3b4ca8); + /** Zero. */ + public static final UnsignedInt128 ZERO = new UnsignedInt128(0L); + + /** One. */ + public static final UnsignedInt128 ONE = new UnsignedInt128(1L); + + /** The TLS scratch array used for computations */ + private static final ThreadLocal scratch = new ThreadLocal() { + @Override + protected int[] initialValue() { + return new int[INT_COUNT * 2]; + } + }; + /** * Int32 elements as little-endian (v[0] is least significant) unsigned * integers. @@ -1807,113 +1821,31 @@ private void shiftLeftDestructive(int wordShifts, int bitShiftsInWord) { * the value to multiply. in */ private static void multiplyArrays4And4To4NoOverflow(int[] left, int[] right) { - assert (left.length == 4); - assert (right.length == 4); - long product; - - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK); - int z0 = (int) product; - - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[1] & SqlMathUtil.LONG_MASK) - + (right[1] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK) + (product >>> 32); - int z1 = (int) product; - - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[2] & SqlMathUtil.LONG_MASK) - + (right[1] & SqlMathUtil.LONG_MASK) - * (left[1] & SqlMathUtil.LONG_MASK) - + (right[2] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK) + (product >>> 32); - int z2 = (int) product; - - // v[3] - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[3] & SqlMathUtil.LONG_MASK) - + (right[1] & SqlMathUtil.LONG_MASK) - * (left[2] & SqlMathUtil.LONG_MASK) - + (right[2] & SqlMathUtil.LONG_MASK) - * (left[1] & SqlMathUtil.LONG_MASK) - + (right[3] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK) + (product >>> 32); - int z3 = (int) product; - if ((product >>> 32) != 0) { - SqlMathUtil.throwOverflowException(); - } + assert (left.length == INT_COUNT); + assert (right.length == INT_COUNT); - // the combinations below definitely result in overflow - if ((right[3] != 0 && (left[3] != 0 || left[2] != 0 || left[1] != 0)) - || (right[2] != 0 && (left[3] != 0 || left[2] != 0)) - || (right[1] != 0 && left[3] != 0)) { - SqlMathUtil.throwOverflowException(); - } + int[] value = multiplyArrays4And4To8(left, right); - left[0] = z0; - left[1] = z1; - left[2] = z2; - left[3] = z3; - } + left[0] = value[0]; + left[1] = value[1]; + left[2] = value[2]; + left[3] = value[3]; + + assert(value[4] == 0); + assert(value[5] == 0); + assert(value[6] == 0); + assert(value[7] == 0); +} private static int[] multiplyArrays4And4To8(int[] left, int[] right) { - assert (left.length == 4); - assert (right.length == 4); - long product; - - // this method could go beyond the integer ranges until we scale back - // so, we need twice more variables. - int[] z = new int[8]; - - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK); - z[0] = (int) product; - - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[1] & SqlMathUtil.LONG_MASK) - + (right[1] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK) + (product >>> 32); - z[1] = (int) product; - - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[2] & SqlMathUtil.LONG_MASK) - + (right[1] & SqlMathUtil.LONG_MASK) - * (left[1] & SqlMathUtil.LONG_MASK) - + (right[2] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK) + (product >>> 32); - z[2] = (int) product; - - product = (right[0] & SqlMathUtil.LONG_MASK) - * (left[3] & SqlMathUtil.LONG_MASK) - + (right[1] & SqlMathUtil.LONG_MASK) - * (left[2] & SqlMathUtil.LONG_MASK) - + (right[2] & SqlMathUtil.LONG_MASK) - * (left[1] & SqlMathUtil.LONG_MASK) - + (right[3] & SqlMathUtil.LONG_MASK) - * (left[0] & SqlMathUtil.LONG_MASK) + (product >>> 32); - z[3] = (int) product; - - product = (right[1] & SqlMathUtil.LONG_MASK) - * (left[3] & SqlMathUtil.LONG_MASK) - + (right[2] & SqlMathUtil.LONG_MASK) - * (left[2] & SqlMathUtil.LONG_MASK) - + (right[3] & SqlMathUtil.LONG_MASK) - * (left[1] & SqlMathUtil.LONG_MASK) + (product >>> 32); - z[4] = (int) product; - - product = (right[2] & SqlMathUtil.LONG_MASK) - * (left[3] & SqlMathUtil.LONG_MASK) - + (right[3] & SqlMathUtil.LONG_MASK) - * (left[2] & SqlMathUtil.LONG_MASK) + (product >>> 32); - z[5] = (int) product; - - // v[1], v[0] - product = (right[3] & SqlMathUtil.LONG_MASK) - * (left[3] & SqlMathUtil.LONG_MASK) + (product >>> 32); - z[6] = (int) product; - z[7] = (int) (product >>> 32); - - return z; + + // Get thread-local scratch array as a place to compute and hold + // the result without having to do an allocation (new() call), to improve performance. + int[] value = scratch.get(); + + // TODO compute left * right and put the result in value + + return value; } private static void incrementArray(int[] array) { diff --git a/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java b/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java index 6824cd7..174321b 100644 --- a/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java +++ b/common/src/test/org/apache/hadoop/hive/common/type/TestDecimal128.java @@ -18,14 +18,13 @@ import static org.junit.Assert.*; import java.math.BigDecimal; -import java.math.MathContext; +import java.math.BigInteger; import java.math.RoundingMode; import java.util.Random; import org.junit.After; import org.junit.Before; import org.junit.Test; - import org.apache.hadoop.hive.common.type.UnsignedInt128; /** @@ -736,8 +735,96 @@ public void testPrecisionOverflow() { } catch (ArithmeticException ex) { } } + + @Test + public void testRegressionHive6399() { + + Decimal128 op1 = new Decimal128("-605044214913338382", (short) 0); + Decimal128 op2 = new Decimal128("55269579109718297360", (short) 0); + Decimal128 expected = new Decimal128("-33440539101030154945490585226577271520", (short) 0); + + Decimal128 result = new Decimal128(); + Decimal128.multiply(op1, op2, result, (short) 0); + + assertEquals(expected, result); + } + + private void doTestUnsignedInt128MultiplyTo256( + UnsignedInt128 left, UnsignedInt128 right) { + + // Compute the products using pure BigInteger operations + BigInteger biLeft = left.toBigIntegerSlow(); + BigInteger biRight = right.toBigIntegerSlow(); + BigInteger biResult = biLeft.multiply(biRight); + + // Compute the product using UnsignedInt128 operation + int[] result = left.multiplyConstructive256(right); + assertEquals (8, result.length); + + // Read the LittleEndian int[] value into a BigInteger + BigInteger bigInt = BigInteger.valueOf(result[result.length-1] & SqlMathUtil.LONG_MASK); + for(int i=result.length-2; i >= 0; --i) { + bigInt = bigInt.shiftLeft(32); + bigInt = bigInt.add(BigInteger.valueOf(result[i] & SqlMathUtil.LONG_MASK)); + } + + // Now compare the two modes of doing the multiply + assertEquals(biResult, bigInt); + } + + + private void doTestOneUnsignedInt128MultiplyTo256(UnsignedInt128 x) { + doTestUnsignedInt128MultiplyTo256(x, UnsignedInt128.MAX_VALUE); + doTestUnsignedInt128MultiplyTo256(x, UnsignedInt128.MIN_VALUE); + doTestUnsignedInt128MultiplyTo256(x, UnsignedInt128.TEN_TO_THIRTYEIGHT); + doTestUnsignedInt128MultiplyTo256(x, UnsignedInt128.ZERO); + doTestUnsignedInt128MultiplyTo256(x, UnsignedInt128.ONE); + doTestUnsignedInt128MultiplyTo256(x, x); + } + + private void doTestComposeIntForUnsignedInt128MultiplyTo256(int x, int f) { + doTestOneUnsignedInt128MultiplyTo256(new UnsignedInt128(x, f, f, f)); + doTestOneUnsignedInt128MultiplyTo256(new UnsignedInt128(x, x, f, f)); + doTestOneUnsignedInt128MultiplyTo256(new UnsignedInt128(x, x, x, f)); + doTestOneUnsignedInt128MultiplyTo256(new UnsignedInt128(x, x, x, x)); + doTestOneUnsignedInt128MultiplyTo256(new UnsignedInt128(f, x, x, x)); + doTestOneUnsignedInt128MultiplyTo256(new UnsignedInt128(f, f, x, x)); + doTestOneUnsignedInt128MultiplyTo256(new UnsignedInt128(f, f, f, x)); + } @Test + public void testUnsignedInt128MultiplyTo256() { + + int[] interestingInts = new int[] { + 0, + 0xFFFFFFFF, + 0x7FFFFFFE, + 0x7FFFFFFF, + 0x80000000, + 0x80000001, + -2, + 1 + }; + + for (int i = 0; i < interestingInts.length; ++i) { + int x = interestingInts[i]; + + doTestComposeIntForUnsignedInt128MultiplyTo256(x, 0); + doTestComposeIntForUnsignedInt128MultiplyTo256(0, x); + doTestComposeIntForUnsignedInt128MultiplyTo256(x, -1); + doTestComposeIntForUnsignedInt128MultiplyTo256(-1, x); + doTestComposeIntForUnsignedInt128MultiplyTo256(x, 1); + doTestComposeIntForUnsignedInt128MultiplyTo256(1, x); + doTestComposeIntForUnsignedInt128MultiplyTo256(x, -2); + doTestComposeIntForUnsignedInt128MultiplyTo256(-2, x); + doTestComposeIntForUnsignedInt128MultiplyTo256(x, 2); + doTestComposeIntForUnsignedInt128MultiplyTo256(2, x); + doTestComposeIntForUnsignedInt128MultiplyTo256(x, 0x80000000); + doTestComposeIntForUnsignedInt128MultiplyTo256(0x80000000, x); + } + } + + @Test public void testToLong() { Decimal128 d = new Decimal128("1.25", (short) 2); assertEquals(1, d.longValue());