Made the reducer for `min()` and `max()` not depend on an Integer seed value. It now uses `Double.NaN` again, but `Double.NaN` also got some special handling in `NumberHelper`, so that results for the reducing steps are not forced to be of type `Double`.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/e237e237 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/e237e237 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/e237e237 Branch: refs/heads/TINKERPOP-1857 Commit: e237e2372269688844c59b956ff7a903747aa1e5 Parents: 0bcab7f Author: Daniel Kuppitz <[email protected]> Authored: Sat Jan 20 17:50:55 2018 -0700 Committer: Daniel Kuppitz <[email protected]> Committed: Tue Jan 30 08:28:57 2018 -0700 ---------------------------------------------------------------------- .../gremlin/process/traversal/NumberHelper.java | 168 +++++++++++++++---- .../traversal/step/map/MaxGlobalStep.java | 2 +- .../traversal/step/map/MinGlobalStep.java | 2 +- .../process/traversal/step/map/MinTest.java | 18 ++ 4 files changed, 154 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e237e237/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 a25916b..789c01e 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 @@ -34,12 +34,24 @@ public class NumberHelper { (a, b) -> a.byteValue() * b.byteValue(), (a, b) -> a.byteValue() / b.byteValue(), (a, b) -> { - final byte x = a.byteValue(), y = b.byteValue(); - return x <= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final byte x = a.byteValue(), y = b.byteValue(); + return x <= y ? x : y; + } + return a.byteValue(); + } + return b.byteValue(); }, (a, b) -> { - final byte x = a.byteValue(), y = b.byteValue(); - return x >= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final byte x = a.byteValue(), y = b.byteValue(); + return x >= y ? x : y; + } + return a.byteValue(); + } + return b.byteValue(); }, (a, b) -> Byte.compare(a.byteValue(), b.byteValue())); @@ -49,12 +61,24 @@ public class NumberHelper { (a, b) -> a.shortValue() * b.shortValue(), (a, b) -> a.shortValue() / b.shortValue(), (a, b) -> { - final short x = a.shortValue(), y = b.shortValue(); - return x <= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final short x = a.shortValue(), y = b.shortValue(); + return x <= y ? x : y; + } + return a.shortValue(); + } + return b.shortValue(); }, (a, b) -> { - final short x = a.shortValue(), y = b.shortValue(); - return x >= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final short x = a.shortValue(), y = b.shortValue(); + return x >= y ? x : y; + } + return a.shortValue(); + } + return b.shortValue(); }, (a, b) -> Short.compare(a.shortValue(), b.shortValue())); @@ -64,12 +88,24 @@ public class NumberHelper { (a, b) -> a.intValue() * b.intValue(), (a, b) -> a.intValue() / b.intValue(), (a, b) -> { - final int x = a.intValue(), y = b.intValue(); - return x <= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final int x = a.intValue(), y = b.intValue(); + return x <= y ? x : y; + } + return a.intValue(); + } + return b.intValue(); }, (a, b) -> { - final int x = a.intValue(), y = b.intValue(); - return x >= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final int x = a.intValue(), y = b.intValue(); + return x >= y ? x : y; + } + return a.intValue(); + } + return b.intValue(); }, (a, b) -> Integer.compare(a.intValue(), b.intValue())); @@ -79,12 +115,24 @@ public class NumberHelper { (a, b) -> a.longValue() * b.longValue(), (a, b) -> a.longValue() / b.longValue(), (a, b) -> { - final long x = a.longValue(), y = b.longValue(); - return x <= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final long x = a.longValue(), y = b.longValue(); + return x <= y ? x : y; + } + return a.longValue(); + } + return b.longValue(); }, (a, b) -> { - final long x = a.longValue(), y = b.longValue(); - return x >= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final long x = a.longValue(), y = b.longValue(); + return x >= y ? x : y; + } + return a.longValue(); + } + return b.longValue(); }, (a, b) -> Long.compare(a.longValue(), b.longValue())); @@ -94,12 +142,24 @@ public class NumberHelper { (a, b) -> bigIntegerValue(a).multiply(bigIntegerValue(b)), (a, b) -> bigIntegerValue(a).divide(bigIntegerValue(b)), (a, b) -> { - final BigInteger x = bigIntegerValue(a), y = bigIntegerValue(b); - return x.compareTo(y) <= 0 ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final BigInteger x = bigIntegerValue(a), y = bigIntegerValue(b); + return x.compareTo(y) <= 0 ? x : y; + } + return bigIntegerValue(a); + } + return bigIntegerValue(b); }, (a, b) -> { - final BigInteger x = bigIntegerValue(a), y = bigIntegerValue(b); - return x.compareTo(y) >= 0 ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final BigInteger x = bigIntegerValue(a), y = bigIntegerValue(b); + return x.compareTo(y) >= 0 ? x : y; + } + return bigIntegerValue(a); + } + return bigIntegerValue(b); }, (a, b) -> bigIntegerValue(a).compareTo(bigIntegerValue(b))); @@ -109,12 +169,24 @@ public class NumberHelper { (a, b) -> a.floatValue() * b.floatValue(), (a, b) -> a.floatValue() / b.floatValue(), (a, b) -> { - final float x = a.floatValue(), y = b.floatValue(); - return x <= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final float x = a.floatValue(), y = b.floatValue(); + return x <= y ? x : y; + } + return a.floatValue(); + } + return b.floatValue(); }, (a, b) -> { - final float x = a.floatValue(), y = b.floatValue(); - return x >= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final float x = a.floatValue(), y = b.floatValue(); + return x >= y ? x : y; + } + return a.floatValue(); + } + return b.floatValue(); }, (a, b) -> Float.compare(a.floatValue(), b.floatValue())); @@ -124,12 +196,24 @@ public class NumberHelper { (a, b) -> a.doubleValue() * b.doubleValue(), (a, b) -> a.doubleValue() / b.doubleValue(), (a, b) -> { - final double x = a.doubleValue(), y = b.doubleValue(); - return x <= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final double x = a.doubleValue(), y = b.doubleValue(); + return x <= y ? x : y; + } + return a.doubleValue(); + } + return b.doubleValue(); }, (a, b) -> { - final double x = a.doubleValue(), y = b.doubleValue(); - return x >= y ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final double x = a.doubleValue(), y = b.doubleValue(); + return x >= y ? x : y; + } + return a.doubleValue(); + } + return b.doubleValue(); }, (a, b) -> Double.compare(a.doubleValue(), b.doubleValue())); @@ -144,7 +228,7 @@ public class NumberHelper { return ba.divide(bb); } catch (ArithmeticException ignored) { // set a default precision - final int precision = Math.max(ba.precision(),bb.precision()) + 10; + 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); @@ -152,12 +236,24 @@ public class NumberHelper { } }, (a, b) -> { - final BigDecimal x = bigDecimalValue(a), y = bigDecimalValue(b); - return x.compareTo(y) <= 0 ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final BigDecimal x = bigDecimalValue(a), y = bigDecimalValue(b); + return x.compareTo(y) <= 0 ? x : y; + } + return bigDecimalValue(a); + } + return bigDecimalValue(b); }, (a, b) -> { - final BigDecimal x = bigDecimalValue(a), y = bigDecimalValue(b); - return x.compareTo(y) >= 0 ? x : y; + if (isNumber(a)) { + if (isNumber(b)) { + final BigDecimal x = bigDecimalValue(a), y = bigDecimalValue(b); + return x.compareTo(y) >= 0 ? x : y; + } + return bigDecimalValue(a); + } + return bigDecimalValue(b); }, (a, b) -> bigDecimalValue(a).compareTo(bigDecimalValue(b))); @@ -194,7 +290,7 @@ public class NumberHelper { int bits = 8; boolean fp = forceFloatingPoint; for (final Number number : numbers) { - if (number == null) continue; + if (!isNumber(number)) continue; final Class<? extends Number> clazz = number.getClass(); if (clazz.equals(Byte.class)) continue; if (clazz.equals(Short.class)) { @@ -315,4 +411,8 @@ public class NumberHelper { return BigInteger.class; } } + + private static boolean isNumber(final Number number) { + return number != null && !number.equals(Double.NaN); + } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e237e237/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxGlobalStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxGlobalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxGlobalStep.java index eee64d0..954dbfe 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxGlobalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxGlobalStep.java @@ -36,7 +36,7 @@ public final class MaxGlobalStep<S extends Number> extends ReducingBarrierStep<S public MaxGlobalStep(final Traversal.Admin traversal) { super(traversal); - this.setSeedSupplier(new ConstantSupplier<>((S) Integer.valueOf(Integer.MIN_VALUE))); + this.setSeedSupplier(new ConstantSupplier<>((S) Double.valueOf(Double.NaN))); this.setReducingBiOperator((BinaryOperator) Operator.max); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e237e237/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinGlobalStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinGlobalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinGlobalStep.java index 5779e97..7d0eb56 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinGlobalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinGlobalStep.java @@ -36,7 +36,7 @@ public final class MinGlobalStep<S extends Number> extends ReducingBarrierStep<S public MinGlobalStep(final Traversal.Admin traversal) { super(traversal); - this.setSeedSupplier(new ConstantSupplier<>((S)Integer.valueOf(Integer.MAX_VALUE))); + this.setSeedSupplier(new ConstantSupplier<>((S) Double.valueOf(Double.NaN))); this.setReducingBiOperator((BinaryOperator) Operator.min); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e237e237/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinTest.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinTest.java index 7e016ed..947137f 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinTest.java @@ -27,6 +27,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import java.util.Arrays; +import java.util.Collections; import java.util.Map; import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN; @@ -46,6 +47,8 @@ public abstract class MinTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, Map<String, Number>> get_g_V_hasLabelXsoftwareX_group_byXnameX_byXbothE_weight_minX(); + public abstract Traversal<Vertex, Number> get_g_V_foo_injectX9999999999X_min(); + @Test @LoadGraphWith(MODERN) public void g_V_age_min() { @@ -75,6 +78,16 @@ public abstract class MinTest extends AbstractGremlinProcessTest { assertEquals(0.2, map.get("lop")); } + @Test + @LoadGraphWith(MODERN) + public void g_V_foo_injectX9999999999X_min() { + final Traversal<Vertex, Number> traversal = get_g_V_foo_injectX9999999999X_min(); + printTraversalForm(traversal); + assertTrue(traversal.hasNext()); + assertEquals(9999999999L, traversal.next().longValue()); + assertFalse(traversal.hasNext()); + } + public static class Traversals extends MinTest { @Override @@ -91,5 +104,10 @@ public abstract class MinTest extends AbstractGremlinProcessTest { public Traversal<Vertex, Map<String, Number>> get_g_V_hasLabelXsoftwareX_group_byXnameX_byXbothE_weight_minX() { return g.V().hasLabel("software").<String, Number>group().by("name").by(bothE().values("weight").min()); } + + @Override + public Traversal<Vertex, Number> get_g_V_foo_injectX9999999999X_min() { + return g.V().values("foo").inject(9999999999L).min(); + } } } \ No newline at end of file
