Repository: phoenix Updated Branches: refs/heads/master 29c610cfd -> e8c2664e7
PHOENIX-1206 Decimal serialization broken for negative numbers with more than 19 digits of precision (Kyle Buzsaki) Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/e8c2664e Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/e8c2664e Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/e8c2664e Branch: refs/heads/master Commit: e8c2664e7705901991114278d182b65c2fb57c00 Parents: 29c610c Author: James Taylor <[email protected]> Authored: Wed Aug 27 18:18:12 2014 -0700 Committer: James Taylor <[email protected]> Committed: Wed Aug 27 18:18:12 2014 -0700 ---------------------------------------------------------------------- .../org/apache/phoenix/schema/PDataType.java | 4 +-- .../org/apache/phoenix/util/NumberUtil.java | 2 +- .../RoundFloorCeilExpressionsTest.java | 31 ++++++++++++-------- .../apache/phoenix/schema/PDataTypeTest.java | 22 ++++++++++++++ 4 files changed, 43 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/e8c2664e/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java index 714028c..3d38d64 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java @@ -6596,7 +6596,7 @@ public enum PDataType { } else { // Adjust length and offset down because we don't have enough room length = MAX_BIG_DECIMAL_BYTES; - index = offset + length - 1; + index = offset + length; } } BigInteger bi = v.unscaledValue(); @@ -6605,7 +6605,7 @@ public enum PDataType { BigInteger[] dandr = bi.divideAndRemainder(divideBy); bi = dandr[0]; int digit = dandr[1].intValue(); - result[--index] = (byte)(signum * digit * multiplyBy + digitOffset); + result[--index] = (byte)(digit * multiplyBy + digitOffset); multiplyBy = 1; divideBy = ONE_HUNDRED; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/e8c2664e/phoenix-core/src/main/java/org/apache/phoenix/util/NumberUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/NumberUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/NumberUtil.java index 1943b6b..7889a89 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/NumberUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/NumberUtil.java @@ -37,7 +37,7 @@ public class NumberUtil { * @return new {@link BigDecimal} instance */ public static BigDecimal normalize(BigDecimal bigDecimal) { - return bigDecimal.stripTrailingZeros().round(PDataType.DEFAULT_MATH_CONTEXT); + return bigDecimal.round(PDataType.DEFAULT_MATH_CONTEXT).stripTrailingZeros(); } public static BigDecimal setDecimalWidthAndScale(BigDecimal decimal, Integer precisionOrNull, Integer scaleOrNull) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/e8c2664e/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java index 25520c4..4757e5b 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/RoundFloorCeilExpressionsTest.java @@ -327,19 +327,24 @@ public class RoundFloorCeilExpressionsTest { // value doesn't matter because we only use those expressions to produce a keypart private static final LiteralExpression DUMMY_DECIMAL = LiteralExpression.newConstant(new BigDecimal("2.5")); - // this should be PDataType#MAX_PRECISION (38) - // but there are rounding errors in DECIMAL.toBytes() and DECIMAL.toObject() - // with precisions of 20 or greater. See https://issues.apache.org/jira/browse/PHOENIX-1206 - private static final int MAX_RELIABLE_PRECISION = 18; - - // once PHOENIX-1206 is fixed, we should add more precise decimals to these tests private static final List<BigDecimal> DECIMALS = Collections.unmodifiableList( Arrays.asList( - new BigDecimal("-200300"), new BigDecimal("-8.44"), new BigDecimal("-2.00"), - new BigDecimal("-0.6"), new BigDecimal("-0.00032"), - BigDecimal.ZERO, BigDecimal.ONE, - new BigDecimal("0.00000984"), new BigDecimal("0.74"), new BigDecimal("2.00"), - new BigDecimal("7.09"), new BigDecimal("84900800") + BigDecimal.valueOf(Long.MIN_VALUE * 17L - 13L, 9), + BigDecimal.valueOf(Long.MIN_VALUE, 8), + new BigDecimal("-200300"), + new BigDecimal("-8.44"), + new BigDecimal("-2.00"), + new BigDecimal("-0.6"), + new BigDecimal("-0.00032"), + BigDecimal.ZERO, + BigDecimal.ONE, + new BigDecimal("0.00000984"), + new BigDecimal("0.74"), + new BigDecimal("2.00"), + new BigDecimal("7.09"), + new BigDecimal("84900800"), + BigDecimal.valueOf(Long.MAX_VALUE, 8), + BigDecimal.valueOf(Long.MAX_VALUE * 31L + 17L, 7) )); private static final List<Integer> SCALES = Collections.unmodifiableList(Arrays.asList(0, 1, 2, 3, 8)); @@ -482,10 +487,10 @@ public class RoundFloorCeilExpressionsTest { * @throws IllegalArgumentException if decimal requires more than the maximum reliable precision */ private static BigDecimal getSmallestUnit(BigDecimal decimal) { - if (decimal.precision() > MAX_RELIABLE_PRECISION) { + if (decimal.precision() > PDataType.MAX_PRECISION) { throw new IllegalArgumentException("rounding errors mean that we cannot reliably test " + decimal); } - int minScale = decimal.scale() + (MAX_RELIABLE_PRECISION - decimal.precision()); + int minScale = decimal.scale() + (PDataType.MAX_PRECISION - decimal.precision()); return BigDecimal.valueOf(1, minScale); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/e8c2664e/phoenix-core/src/test/java/org/apache/phoenix/schema/PDataTypeTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/schema/PDataTypeTest.java b/phoenix-core/src/test/java/org/apache/phoenix/schema/PDataTypeTest.java index d6709b6..e952d8b 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/schema/PDataTypeTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/schema/PDataTypeTest.java @@ -848,6 +848,28 @@ public class PDataTypeTest { nb = (BigDecimal)PDataType.DECIMAL.toObject(b); TestUtil.assertRoundEquals(na,nb); assertTrue(b.length <= PDataType.DECIMAL.estimateByteSize(na)); + + // test for negative serialization using biginteger + na = new BigDecimal("-5.00000000000000000000000001"); + b = PDataType.DECIMAL.toBytes(na); + nb = (BigDecimal)PDataType.DECIMAL.toObject(b); + TestUtil.assertRoundEquals(na,nb); + assertTrue(b.length <= PDataType.DECIMAL.estimateByteSize(na)); + + // test for serialization of 38 digits + na = new BigDecimal("-2.4999999999999999999999999999999999999"); + b = PDataType.DECIMAL.toBytes(na); + nb = (BigDecimal)PDataType.DECIMAL.toObject(b); + TestUtil.assertRoundEquals(na,nb); + assertTrue(b.length <= PDataType.DECIMAL.estimateByteSize(na)); + + // test for serialization of 39 digits, should round to -2.5 + na = new BigDecimal("-2.499999999999999999999999999999999999999"); + b = PDataType.DECIMAL.toBytes(na); + nb = (BigDecimal)PDataType.DECIMAL.toObject(b); + assertTrue(nb.compareTo(new BigDecimal("-2.5")) == 0); + assertEquals(new BigDecimal("-2.5"), nb); + assertTrue(b.length <= PDataType.DECIMAL.estimateByteSize(na)); na = new BigDecimal(2.5); b = PDataType.DECIMAL.toBytes(na);
