Fixed ordering of collections / streams containing mixed number types.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/9bb66617 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/9bb66617 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/9bb66617 Branch: refs/heads/TINKERPOP-1759 Commit: 9bb66617293fb9e89030feae3f430b67fb3e9aa9 Parents: 0048591 Author: Daniel Kuppitz <[email protected]> Authored: Thu Aug 17 06:37:45 2017 -0700 Committer: Daniel Kuppitz <[email protected]> Committed: Fri Aug 25 12:27:55 2017 -0700 ---------------------------------------------------------------------- .../gremlin/process/traversal/NumberHelper.java | 34 ++++++++--- .../gremlin/process/traversal/Order.java | 6 ++ .../traversal/step/map/GroovyOrderTest.groovy | 10 ++++ .../process/traversal/step/map/OrderTest.java | 59 ++++++++++++++++++++ 4 files changed, 100 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9bb66617/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 eec1d9e..a25916b 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 @@ -40,7 +40,8 @@ public class NumberHelper { (a, b) -> { final byte x = a.byteValue(), y = b.byteValue(); return x >= y ? x : y; - }); + }, + (a, b) -> Byte.compare(a.byteValue(), b.byteValue())); static final NumberHelper SHORT_NUMBER_HELPER = new NumberHelper( (a, b) -> a.shortValue() + b.shortValue(), @@ -54,7 +55,8 @@ public class NumberHelper { (a, b) -> { final short x = a.shortValue(), y = b.shortValue(); return x >= y ? x : y; - }); + }, + (a, b) -> Short.compare(a.shortValue(), b.shortValue())); static final NumberHelper INTEGER_NUMBER_HELPER = new NumberHelper( (a, b) -> a.intValue() + b.intValue(), @@ -68,7 +70,8 @@ public class NumberHelper { (a, b) -> { final int x = a.intValue(), y = b.intValue(); return x >= y ? x : y; - }); + }, + (a, b) -> Integer.compare(a.intValue(), b.intValue())); static final NumberHelper LONG_NUMBER_HELPER = new NumberHelper( (a, b) -> a.longValue() + b.longValue(), @@ -82,7 +85,8 @@ public class NumberHelper { (a, b) -> { final long x = a.longValue(), y = b.longValue(); return x >= y ? x : y; - }); + }, + (a, b) -> Long.compare(a.longValue(), b.longValue())); static final NumberHelper BIG_INTEGER_NUMBER_HELPER = new NumberHelper( (a, b) -> bigIntegerValue(a).add(bigIntegerValue(b)), @@ -96,7 +100,8 @@ public class NumberHelper { (a, b) -> { final BigInteger x = bigIntegerValue(a), y = bigIntegerValue(b); return x.compareTo(y) >= 0 ? x : y; - }); + }, + (a, b) -> bigIntegerValue(a).compareTo(bigIntegerValue(b))); static final NumberHelper FLOAT_NUMBER_HELPER = new NumberHelper( (a, b) -> a.floatValue() + b.floatValue(), @@ -110,7 +115,8 @@ public class NumberHelper { (a, b) -> { final float x = a.floatValue(), y = b.floatValue(); return x >= y ? x : y; - }); + }, + (a, b) -> Float.compare(a.floatValue(), b.floatValue())); static final NumberHelper DOUBLE_NUMBER_HELPER = new NumberHelper( (a, b) -> a.doubleValue() + b.doubleValue(), @@ -124,7 +130,8 @@ public class NumberHelper { (a, b) -> { final double x = a.doubleValue(), y = b.doubleValue(); return x >= y ? x : y; - }); + }, + (a, b) -> Double.compare(a.doubleValue(), b.doubleValue())); static final NumberHelper BIG_DECIMAL_NUMBER_HELPER = new NumberHelper( (a, b) -> bigDecimalValue(a).add(bigDecimalValue(b)), @@ -151,7 +158,8 @@ public class NumberHelper { (a, b) -> { final BigDecimal x = bigDecimalValue(a), y = bigDecimalValue(b); return x.compareTo(y) >= 0 ? x : y; - }); + }, + (a, b) -> bigDecimalValue(a).compareTo(bigDecimalValue(b))); public final BiFunction<Number, Number, Number> add; public final BiFunction<Number, Number, Number> sub; @@ -159,13 +167,15 @@ public class NumberHelper { public final BiFunction<Number, Number, Number> div; public final BiFunction<Number, Number, Number> min; public final BiFunction<Number, Number, Number> max; + public final BiFunction<Number, Number, Integer> cmp; private NumberHelper(final BiFunction<Number, Number, Number> add, final BiFunction<Number, Number, Number> sub, final BiFunction<Number, Number, Number> mul, final BiFunction<Number, Number, Number> div, final BiFunction<Number, Number, Number> min, - final BiFunction<Number, Number, Number> max + final BiFunction<Number, Number, Number> max, + final BiFunction<Number, Number, Integer> cmp ) { this.add = add; this.sub = sub; @@ -173,6 +183,7 @@ public class NumberHelper { this.div = div; this.min = min; this.max = max; + this.cmp = cmp; } public static Class<? extends Number> getHighestCommonNumberClass(final Number... numbers) { @@ -243,6 +254,11 @@ public class NumberHelper { return getHelper(clazz).max.apply(a, b); } + public static Integer compare(final Number a, final Number b) { + final Class<? extends Number> clazz = getHighestCommonNumberClass(a, b); + return getHelper(clazz).cmp.apply(a, b); + } + private static NumberHelper getHelper(final Class<? extends Number> clazz) { if (clazz.equals(Byte.class)) { return BYTE_NUMBER_HELPER; http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9bb66617/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java index 4e3801a..3a3ae2b 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Order.java @@ -30,6 +30,9 @@ public enum Order implements Comparator<Object> { incr { @Override public int compare(final Object first, final Object second) { + if (first instanceof Number && second instanceof Number) { + return NumberHelper.compare((Number) first, (Number) second); + } return Comparator.<Comparable>naturalOrder().compare((Comparable) first, (Comparable) second); } @@ -40,6 +43,9 @@ public enum Order implements Comparator<Object> { }, decr { @Override public int compare(final Object first, final Object second) { + if (first instanceof Number && second instanceof Number) { + return NumberHelper.compare((Number) second, (Number) first); + } return Comparator.<Comparable>reverseOrder().compare((Comparable) first, (Comparable) second); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9bb66617/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyOrderTest.groovy ---------------------------------------------------------------------- diff --git a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyOrderTest.groovy b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyOrderTest.groovy index de33944..7708f1a 100644 --- a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyOrderTest.groovy +++ b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyOrderTest.groovy @@ -136,5 +136,15 @@ public abstract class GroovyOrderTest { public Traversal<Vertex, String> get_g_V_hasLabelXsongX_order_byXperfomances_decrX_byXnameX_rangeX110_120X_name() { new ScriptTraversal<>(g, "gremlin-groovy", "g.V.hasLabel('song').order.by('performances',decr).by('name').range(110, 120).name") } + + @Override + public Traversal<Vertex, Map<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_orderXlocalX_byXvaluesX() { + new ScriptTraversal<>(g, "gremlin-groovy", "g.V.hasLabel('person').group.by('name').by(__.outE.weight.sum).order(local).by(values)") + } + + @Override + public Traversal<Vertex, Map.Entry<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_unfold_order_byXvalues_decrX() { + new ScriptTraversal<>(g, "gremlin-groovy", "g.V.hasLabel('person').group.by('name').by(__.outE.weight.sum).unfold.order.by(values, decr)") + } } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9bb66617/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java index 7935252..bbf63ef 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/OrderTest.java @@ -96,6 +96,10 @@ public abstract class OrderTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, String> get_g_V_hasLabelXsongX_order_byXperfomances_decrX_byXnameX_rangeX110_120X_name(); + public abstract Traversal<Vertex, Map<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_orderXlocalX_byXvaluesX(); + + public abstract Traversal<Vertex, Map.Entry<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_unfold_order_byXvalues_decrX(); + @Test @LoadGraphWith(MODERN) public void g_V_name_order() { @@ -374,6 +378,51 @@ public abstract class OrderTest extends AbstractGremlinProcessTest { "KNOCKING ON HEAVENS DOOR", "MEMPHIS BLUES"), traversal); } + @Test + @LoadGraphWith(MODERN) + public void g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_orderXlocalX_byXvaluesX() { + final Traversal<Vertex, Map<String, Number>> traversal = get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_orderXlocalX_byXvaluesX(); + printTraversalForm(traversal); + assertTrue(traversal.hasNext()); + final Map<String, Number> m = traversal.next(); + assertFalse(traversal.hasNext()); + assertEquals(4, m.size()); + final Iterator<Map.Entry<String, Number>> iterator = m.entrySet().iterator(); + Map.Entry<String, Number> entry = iterator.next(); + assertEquals("vadas", entry.getKey()); + assertEquals(0.0, entry.getValue().doubleValue(), 0.0001); + entry = iterator.next(); + assertEquals("peter", entry.getKey()); + assertEquals(0.2, entry.getValue().doubleValue(), 0.0001); + entry = iterator.next(); + assertEquals("josh", entry.getKey()); + assertEquals(1.4, entry.getValue().doubleValue(), 0.0001); + entry = iterator.next(); + assertEquals("marko", entry.getKey()); + assertEquals(1.9, entry.getValue().doubleValue(), 0.0001); + } + + @Test + @LoadGraphWith(MODERN) + public void g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_unfold_order_byXvalues_decrX() { + final Traversal<Vertex, Map.Entry<String, Number>> traversal = get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_unfold_order_byXvalues_decrX(); + printTraversalForm(traversal); + assertTrue(traversal.hasNext()); + Map.Entry<String, Number> entry = traversal.next(); + assertEquals("marko", entry.getKey()); + assertEquals(1.9, entry.getValue().doubleValue(), 0.0001); + entry = traversal.next(); + assertEquals("josh", entry.getKey()); + assertEquals(1.4, entry.getValue().doubleValue(), 0.0001); + entry = traversal.next(); + assertEquals("peter", entry.getKey()); + assertEquals(0.2, entry.getValue().doubleValue(), 0.0001); + entry = traversal.next(); + assertEquals("vadas", entry.getKey()); + assertEquals(0.0, entry.getValue().doubleValue(), 0.0001); + assertFalse(traversal.hasNext()); + } + public static class Traversals extends OrderTest { @Override @@ -484,5 +533,15 @@ public abstract class OrderTest extends AbstractGremlinProcessTest { public Traversal<Vertex, String> get_g_V_hasLabelXsongX_order_byXperfomances_decrX_byXnameX_rangeX110_120X_name() { return g.V().hasLabel("song").order().by("performances", Order.decr).by("name").range(110, 120).values("name"); } + + @Override + public Traversal<Vertex, Map<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_orderXlocalX_byXvaluesX() { + return g.V().hasLabel("person").<String, Number>group().by("name").by(outE().values("weight").sum()).order(Scope.local).by(Column.values); + } + + @Override + public Traversal<Vertex, Map.Entry<String, Number>> get_g_V_hasLabelXpersonX_group_byXnameX_byXoutE_weight_sumX_unfold_order_byXvalues_decrX() { + return g.V().hasLabel("person").group().by("name").by(outE().values("weight").sum()).<Map.Entry<String, Number>>unfold().order().by(Column.values, Order.decr); + } } }
