You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2014/08/28 03:18:26 UTC

git commit: PHOENIX-1206 Decimal serialization broken for negative numbers with more than 19 digits of precision (Kyle Buzsaki)

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 <ja...@apache.org>
Authored: Wed Aug 27 18:18:12 2014 -0700
Committer: James Taylor <ja...@apache.org>
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);