You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by ok...@apache.org on 2015/08/31 22:45:59 UTC

[1/4] incubator-tinkerpop git commit: TINKERPOP3-750: Test large number comparison

Repository: incubator-tinkerpop
Updated Branches:
  refs/heads/tp30 0ee83647f -> 2ffde6c4c


TINKERPOP3-750: Test large number comparison

Repro for comparisons that are sensitive to loss of precision when converting Number to double.


Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/e1917173
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/e1917173
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/e1917173

Branch: refs/heads/tp30
Commit: e1917173d8a44b97799ecffb405948e0549af6d6
Parents: a20d060
Author: mhfrantz <mf...@redsealnetworks.com>
Authored: Mon Aug 31 11:42:35 2015 -0700
Committer: mhfrantz <mf...@redsealnetworks.com>
Committed: Mon Aug 31 13:26:05 2015 -0700

----------------------------------------------------------------------
 .../gremlin/process/traversal/CompareTest.java  | 49 ++++++++++++++++----
 1 file changed, 40 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/e1917173/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java
index 6184c94..a7d8e1b 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/CompareTest.java
@@ -32,13 +32,13 @@ import static org.junit.Assert.assertEquals;
 
 /**
  * @author Stephen Mallette (http://stephen.genoprime.com)
+ * @author Matt Frantz (http://github.com/mhfrantz)
  */
 @RunWith(Parameterized.class)
 public class CompareTest {
 
     @Parameterized.Parameters(name = "{0}.test({1},{2}) = {3}")
     public static Iterable<Object[]> data() {
-        final List<Object> one = Arrays.asList(1, 1l, 1d, 1f, BigDecimal.ONE, BigInteger.ONE);
         final List<Object[]> testCases = new ArrayList<>(Arrays.asList(new Object[][]{
                 {Compare.eq, null, null, true},
                 {Compare.eq, null, 1, false},
@@ -89,16 +89,47 @@ public class CompareTest {
                 {Compare.lte, "z", "a", false},
                 {Compare.lte, "a", "z", true}
         }));
-        for (int i = 0; i < one.size(); i++) {
-            for (int j = 0; j < one.size(); j++) {
-                testCases.add(new Object[]{Compare.eq, one.get(i), one.get(j), true});
-                testCases.add(new Object[]{Compare.neq, one.get(i), one.get(j), false});
-                testCases.add(new Object[]{Compare.gt, one.get(i), one.get(j), false});
-                testCases.add(new Object[]{Compare.lt, one.get(i), one.get(j), false});
-                testCases.add(new Object[]{Compare.gte, one.get(i), one.get(j), true});
-                testCases.add(new Object[]{Compare.lte, one.get(i), one.get(j), true});
+        // Compare Numbers of mixed types.
+        final List<Object> one = Arrays.asList(1, 1l, 1d, 1f, BigDecimal.ONE, BigInteger.ONE);
+        for (Object i : one) {
+            for (Object j : one) {
+                testCases.addAll(Arrays.asList(new Object[][]{
+                            {Compare.eq, i, j, true},
+                            {Compare.neq, i, j, false},
+                            {Compare.gt, i, j, false},
+                            {Compare.lt, i, j, false},
+                            {Compare.gte, i, j, true},
+                            {Compare.lte, i, j, true},
+                        }));
             }
         }
+        // Compare large numbers of different types that cannot convert to doubles losslessly.
+        final BigInteger big1 = new BigInteger("123456789012345678901234567890");
+        final BigDecimal big1d = new BigDecimal("123456789012345678901234567890");
+        final BigDecimal big2 = new BigDecimal(big1.add(BigInteger.ONE));
+        testCases.addAll(Arrays.asList(new Object[][]{
+                    // big1 == big1d
+                    {Compare.eq, big1, big1d, true},
+                    {Compare.neq, big1, big1d, false},
+                    {Compare.gt, big1, big1d, false},
+                    {Compare.lt, big1, big1d, false},
+                    {Compare.gte, big1, big1d, true},
+                    {Compare.lte, big1, big1d, true},
+                    // big1 < big2
+                    {Compare.eq, big1, big2, false},
+                    {Compare.neq, big1, big2, true},
+                    {Compare.gt, big1, big2, false},
+                    {Compare.lt, big1, big2, true},
+                    {Compare.gte, big1, big2, false},
+                    {Compare.lte, big1, big2, true},
+                    // Reverse the operands for symmetric test coverage (big2 > big1)
+                    {Compare.eq, big2, big1, false},
+                    {Compare.neq, big2, big1, true},
+                    {Compare.gt, big2, big1, true},
+                    {Compare.lt, big2, big1, false},
+                    {Compare.gte, big2, big1, true},
+                    {Compare.lte, big2, big1, false},
+                }));
         return testCases;
     }
 


[2/4] incubator-tinkerpop git commit: TINKERPOP3-750: Fix bug in Compare for BigInteger/BigDecimal by converting Number to BigDecimal

Posted by ok...@apache.org.
TINKERPOP3-750: Fix bug in Compare for BigInteger/BigDecimal by converting Number to BigDecimal


Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/378dd6cb
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/378dd6cb
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/378dd6cb

Branch: refs/heads/tp30
Commit: 378dd6cb920c9ac82ce53cc599882bf0704a3ab6
Parents: e191717
Author: mhfrantz <mf...@redsealnetworks.com>
Authored: Mon Aug 31 13:25:40 2015 -0700
Committer: mhfrantz <mf...@redsealnetworks.com>
Committed: Mon Aug 31 13:26:06 2015 -0700

----------------------------------------------------------------------
 .../gremlin/process/traversal/Compare.java      | 47 ++++++++++++--------
 1 file changed, 28 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/378dd6cb/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
index aad8371..262347f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
@@ -18,6 +18,7 @@
  */
 package org.apache.tinkerpop.gremlin.process.traversal;
 
+import java.math.BigDecimal;
 import java.util.function.BiPredicate;
 
 /**
@@ -26,12 +27,13 @@ import java.util.function.BiPredicate;
  *
  * @author Marko A. Rodriguez (http://markorodriguez.com)
  * @author Stephen Mallette (http://stephen.genoprime.com)
+ * @author Matt Frantz (http://github.com/mhfrantz)
  */
 public enum Compare implements BiPredicate<Object, Object> {
     /**
      * Evaluates if the first object is equal to the second.  If both are of type {@link Number} but not of the
-     * same class (i.e. double for the first object and long for the second object) both values are cast to
-     * {@link Number} so that it can be evaluated via {@link Number#doubleValue()}.  Otherwise they are evaluated
+     * same class (i.e. double for the first object and long for the second object) both values are converted to
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
      * via {@link Object#equals(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
@@ -40,7 +42,7 @@ public enum Compare implements BiPredicate<Object, Object> {
         public boolean test(final Object first, final Object second) {
             return null == first ? null == second : (first instanceof Number && second instanceof Number
                     && !first.getClass().equals(second.getClass())
-                    ? ((Number) first).doubleValue() == ((Number) second).doubleValue()
+                    ? big((Number) first).compareTo(big((Number) second)) == 0
                     : first.equals(second));
         }
 
@@ -55,8 +57,8 @@ public enum Compare implements BiPredicate<Object, Object> {
 
     /**
      * Evaluates if the first object is not equal to the second.  If both are of type {@link Number} but not of the
-     * same class (i.e. double for the first object and long for the second object) both values are cast to
-     * {@link Number} so that it can be evaluated via {@link Number#doubleValue()}.  Otherwise they are evaluated
+     * same class (i.e. double for the first object and long for the second object) both values are converted to
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#equals()}.  Otherwise they are evaluated
      * via {@link Object#equals(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
@@ -77,9 +79,9 @@ public enum Compare implements BiPredicate<Object, Object> {
 
     /**
      * Evaluates if the first object is greater than the second.  If both are of type {@link Number} but not of the
-     * same class (i.e. double for the first object and long for the second object) both values are cast to
-     * {@link Number} so that it can be evaluated via {@link Number#doubleValue()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
+     * same class (i.e. double for the first object and long for the second object) both values are converted to
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     gt {
@@ -87,7 +89,7 @@ public enum Compare implements BiPredicate<Object, Object> {
         public boolean test(final Object first, final Object second) {
             return null != first && null != second && (
                     first instanceof Number && second instanceof Number && !first.getClass().equals(second.getClass())
-                            ? ((Number) first).doubleValue() > ((Number) second).doubleValue()
+                            ? big((Number) first).compareTo(big((Number) second)) > 0
                             : ((Comparable) first).compareTo(second) > 0);
         }
 
@@ -102,9 +104,9 @@ public enum Compare implements BiPredicate<Object, Object> {
 
     /**
      * Evaluates if the first object is greater-equal to the second.  If both are of type {@link Number} but not of the
-     * same class (i.e. double for the first object and long for the second object) both values are cast to
-     * {@link Number} so that it can be evaluated via {@link Number#doubleValue()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
+     * same class (i.e. double for the first object and long for the second object) both values are converted to
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     gte {
@@ -124,9 +126,9 @@ public enum Compare implements BiPredicate<Object, Object> {
 
     /**
      * Evaluates if the first object is less than the second.  If both are of type {@link Number} but not of the
-     * same class (i.e. double for the first object and long for the second object) both values are cast to
-     * {@link Number} so that it can be evaluated via {@link Number#doubleValue()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
+     * same class (i.e. double for the first object and long for the second object) both values are converted to
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     lt {
@@ -134,7 +136,7 @@ public enum Compare implements BiPredicate<Object, Object> {
         public boolean test(final Object first, final Object second) {
             return null != first && null != second && (
                     first instanceof Number && second instanceof Number && !first.getClass().equals(second.getClass())
-                            ? ((Number) first).doubleValue() < ((Number) second).doubleValue()
+                            ? big((Number) first).compareTo(big((Number) second)) < 0
                             : ((Comparable) first).compareTo(second) < 0);
         }
 
@@ -149,9 +151,9 @@ public enum Compare implements BiPredicate<Object, Object> {
 
     /**
      * Evaluates if the first object is less-equal to the second.  If both are of type {@link Number} but not of the
-     * same class (i.e. double for the first object and long for the second object) both values are cast to
-     * {@link Number} so that it can be evaluated via {@link Number#doubleValue()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
+     * same class (i.e. double for the first object and long for the second object) both values are converted to
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     lte {
@@ -174,4 +176,11 @@ public enum Compare implements BiPredicate<Object, Object> {
      */
     @Override
     public abstract Compare negate();
+
+    /**
+     * Convert Number to BigDecimal.
+     */
+    private static BigDecimal big(final Number n) {
+        return new BigDecimal(n.toString());
+    }
 }


[3/4] incubator-tinkerpop git commit: Merge branch 'TINKERPOP3-750-Compare-Number-as-BigDecimal' of https://github.com/RedSeal-co/incubator-tinkerpop into tp30

Posted by ok...@apache.org.
Merge branch 'TINKERPOP3-750-Compare-Number-as-BigDecimal' of https://github.com/RedSeal-co/incubator-tinkerpop into tp30


Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/0a11ae1c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/0a11ae1c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/0a11ae1c

Branch: refs/heads/tp30
Commit: 0a11ae1c288f875f9330ad0083ea0ee4e7507344
Parents: 0ee8364 378dd6c
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Mon Aug 31 14:38:58 2015 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Mon Aug 31 14:38:58 2015 -0600

----------------------------------------------------------------------
 .../gremlin/process/traversal/Compare.java      | 47 +++++++++++--------
 .../gremlin/process/traversal/CompareTest.java  | 49 ++++++++++++++++----
 2 files changed, 68 insertions(+), 28 deletions(-)
----------------------------------------------------------------------



[4/4] incubator-tinkerpop git commit: CHANGELOG update and JavaDoc fixes for @mhfrantz Compare BigDecimal fix.

Posted by ok...@apache.org.
CHANGELOG update and JavaDoc fixes for @mhfrantz Compare BigDecimal fix.


Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/2ffde6c4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/2ffde6c4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/2ffde6c4

Branch: refs/heads/tp30
Commit: 2ffde6c4c5ffc743d155ee3ba132988e5cfe964d
Parents: 0a11ae1
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Mon Aug 31 14:46:03 2015 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Mon Aug 31 14:46:03 2015 -0600

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../gremlin/process/traversal/Compare.java      | 20 ++++++++++----------
 2 files changed, 11 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/2ffde6c4/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 4ee6bd6..d20c2fc 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -25,6 +25,7 @@ image::http://www.tinkerpop.com/docs/current/images/gremlin-hindu.png[width=225]
 TinkerPop 3.0.1 (NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* `Compare` now uses `BigDecimal` internally to ensure that precision is not lost on standard number comparisons.
 * Renamed `ComputerVerificationStrategy` to `VerificationStrategy` so all the verification strategies can use it.
 * Added `StandardVerificationStrategy` that throws exceptions for illegal traversal patterns on the standard engine (which extends to `GraphComputer`).
 * Clarified semantics of `Transaction.close()` in unit tests - now refers only to closing the current transaction in the current thread.

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/2ffde6c4/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
index 262347f..97b52b8 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Compare.java
@@ -33,7 +33,7 @@ public enum Compare implements BiPredicate<Object, Object> {
     /**
      * Evaluates if the first object is equal to the second.  If both are of type {@link Number} but not of the
      * same class (i.e. double for the first object and long for the second object) both values are converted to
-     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo}.  Otherwise they are evaluated
      * via {@link Object#equals(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
@@ -58,7 +58,7 @@ public enum Compare implements BiPredicate<Object, Object> {
     /**
      * Evaluates if the first object is not equal to the second.  If both are of type {@link Number} but not of the
      * same class (i.e. double for the first object and long for the second object) both values are converted to
-     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#equals()}.  Otherwise they are evaluated
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#equals}.  Otherwise they are evaluated
      * via {@link Object#equals(Object)}.  Testing against {@link Number#doubleValue()} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
@@ -80,8 +80,8 @@ public enum Compare implements BiPredicate<Object, Object> {
     /**
      * Evaluates if the first object is greater than the second.  If both are of type {@link Number} but not of the
      * same class (i.e. double for the first object and long for the second object) both values are converted to
-     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     gt {
@@ -105,8 +105,8 @@ public enum Compare implements BiPredicate<Object, Object> {
     /**
      * Evaluates if the first object is greater-equal to the second.  If both are of type {@link Number} but not of the
      * same class (i.e. double for the first object and long for the second object) both values are converted to
-     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     gte {
@@ -127,8 +127,8 @@ public enum Compare implements BiPredicate<Object, Object> {
     /**
      * Evaluates if the first object is less than the second.  If both are of type {@link Number} but not of the
      * same class (i.e. double for the first object and long for the second object) both values are converted to
-     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     lt {
@@ -152,8 +152,8 @@ public enum Compare implements BiPredicate<Object, Object> {
     /**
      * Evaluates if the first object is less-equal to the second.  If both are of type {@link Number} but not of the
      * same class (i.e. double for the first object and long for the second object) both values are converted to
-     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo()}.  Otherwise they are evaluated
-     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo()} enables the compare
+     * {@link BigDecimal} so that it can be evaluated via {@link BigDecimal#compareTo}.  Otherwise they are evaluated
+     * via {@link Comparable#compareTo(Object)}.  Testing against {@link BigDecimal#compareTo} enables the compare
      * operations to be a bit more forgiving with respect to comparing different number types.
      */
     lte {