You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by dk...@apache.org on 2017/07/25 20:28:03 UTC

tinkerpop git commit: Fixed a bug in `BigDecimal` divisions in `NumberHelper` that potentially threw an `ArithmeticException`. [Forced Update!]

Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1736 0443211ee -> a612f5f1f (forced update)


Fixed a bug in `BigDecimal` divisions in `NumberHelper` that potentially threw an `ArithmeticException`.


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

Branch: refs/heads/TINKERPOP-1736
Commit: a612f5f1f642555615494168c392c7c135bd67bc
Parents: 28ab372
Author: Daniel Kuppitz <da...@hotmail.com>
Authored: Tue Jul 25 20:01:21 2017 +0200
Committer: Daniel Kuppitz <da...@hotmail.com>
Committed: Tue Jul 25 22:27:47 2017 +0200

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  6 ++++++
 .../gremlin/process/traversal/NumberHelper.java | 16 ++++++++++++++-
 .../step/sideEffect/GroovySackTest.groovy       |  5 +++++
 .../traversal/step/sideEffect/SackTest.java     | 21 ++++++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a612f5f1/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 5436fd9..e2b849c 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -27,6 +27,12 @@ TinkerPop 3.1.8 (Release Date: NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 * Fixed a `MessageScope` bug in `TinkerGraphComputer`.
+* Fixed a bug in `BigDecimal` divisions in `NumberHelper` that potentially threw an `ArithmeticException`.
+
+Bugs
+^^^^
+
+* TINKERPOP-1736 Sack step evaluated by Groovy interprets numbers in an unexpected way
 
 [[release-3-1-7]]
 TinkerPop 3.1.7 (Release Date: June 12, 2017)

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a612f5f1/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/NumberHelper.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/NumberHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/NumberHelper.java
index b094f29..eec1d9e 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/NumberHelper.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/NumberHelper.java
@@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal;
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.math.MathContext;
 import java.util.function.BiFunction;
 
 /**
@@ -129,7 +130,20 @@ public class NumberHelper {
             (a, b) -> bigDecimalValue(a).add(bigDecimalValue(b)),
             (a, b) -> bigDecimalValue(a).subtract(bigDecimalValue(b)),
             (a, b) -> bigDecimalValue(a).multiply(bigDecimalValue(b)),
-            (a, b) -> bigDecimalValue(a).divide(bigDecimalValue(b)),
+            (a, b) -> {
+                final BigDecimal ba = bigDecimalValue(a);
+                final BigDecimal bb = bigDecimalValue(b);
+                try {
+                    return ba.divide(bb);
+                } catch (ArithmeticException ignored) {
+                    // set a default precision
+                    final int precision = Math.max(ba.precision(),bb.precision()) + 10;
+                    BigDecimal result = ba.divide(bb, new MathContext(precision));
+                    final int scale = Math.max(Math.max(ba.scale(), bb.scale()), 10);
+                    if (result.scale() > scale) result = result.setScale(scale, BigDecimal.ROUND_HALF_UP);
+                    return result;
+                }
+            },
             (a, b) -> {
                 final BigDecimal x = bigDecimalValue(a), y = bigDecimalValue(b);
                 return x.compareTo(y) <= 0 ? x : y;

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a612f5f1/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovySackTest.groovy
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovySackTest.groovy b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovySackTest.groovy
index 828f664..d1681cb 100644
--- a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovySackTest.groovy
+++ b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroovySackTest.groovy
@@ -74,5 +74,10 @@ public abstract class GroovySackTest {
         Traversal<Vertex, BigDecimal> get_g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack() {
             TraversalScriptHelper.compute("g.withSack(BigInteger.TEN.pow(1000), assign).V.local(out('knows').barrier(normSack)).in('knows').barrier.sack", g)
         }
+
+        @Override
+        Traversal<Vertex, BigDecimal> get_g_withSackX2X_V_sackXdivX_byXconstantXBigDecimal_valueOfX3XXX_sack() {
+            TraversalScriptHelper.compute("g.withSack(2).V.sack(div).by(constant(3.0)).sack", g)
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a612f5f1/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackTest.java
index 299bee2..63f3649 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackTest.java
@@ -36,6 +36,7 @@ import java.util.HashMap;
 import java.util.Map;
 
 import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.constant;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.outE;
 import static org.junit.Assert.assertEquals;
@@ -68,6 +69,8 @@ public abstract class SackTest extends AbstractGremlinProcessTest {
 
     public abstract Traversal<Vertex, BigDecimal> get_g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack();
 
+    public abstract Traversal<Vertex, BigDecimal> get_g_withSackX2X_V_sackXdivX_byXconstantXBigDecimal_valueOfX3XXX_sack();
+
     @Test
     @LoadGraphWith(MODERN)
     public void g_withSackXhellowX_V_outE_sackXassignX_byXlabelX_inV_sack() {
@@ -156,6 +159,19 @@ public abstract class SackTest extends AbstractGremlinProcessTest {
         assertFalse(traversal.hasNext());
     }
 
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_withSackX2X_V_sackXdivX_byXconstantXBigDecimal_valueOfX3XXX_sack() {
+        final Traversal<Vertex, BigDecimal> traversal = get_g_withSackX2X_V_sackXdivX_byXconstantXBigDecimal_valueOfX3XXX_sack();
+        printTraversalForm(traversal);
+        final double expected = 2.0 / 3.0;
+        for (int i = 0; i < 6; i++) {
+            assertTrue(traversal.hasNext());
+            assertEquals(expected, traversal.next().doubleValue(), 0.0001);
+        }
+        assertFalse(traversal.hasNext());
+    }
+
     public static class Traversals extends SackTest {
 
         @Override
@@ -205,5 +221,10 @@ public abstract class SackTest extends AbstractGremlinProcessTest {
         public Traversal<Vertex, BigDecimal> get_g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack() {
             return g.withSack(BigInteger.TEN.pow(1000), Operator.assign).V().local(out("knows").barrier(SackFunctions.Barrier.normSack)).in("knows").barrier().sack();
         }
+
+        @Override
+        public Traversal<Vertex, BigDecimal> get_g_withSackX2X_V_sackXdivX_byXconstantXBigDecimal_valueOfX3XXX_sack() {
+            return g.withSack(2).V().sack(Operator.div).by(constant(BigDecimal.valueOf(3))).sack();
+        }
     }
 }
\ No newline at end of file