TINKERPOP-1888: Extended `min()` and `max()` to support all comparable data types.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/a1b0ed6e Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/a1b0ed6e Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/a1b0ed6e Branch: refs/heads/TINKERPOP-1888 Commit: a1b0ed6e8f3a1904a9ff98eabea5f0cbd78d3de3 Parents: 46b743d Author: Daniel Kuppitz <daniel_kupp...@hotmail.com> Authored: Mon Apr 2 14:55:27 2018 -0700 Committer: Daniel Kuppitz <daniel_kupp...@hotmail.com> Committed: Mon Apr 9 08:55:56 2018 -0700 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + docs/src/reference/the-traversal.asciidoc | 16 ++++--- docs/src/upgrade/release-3.4.x.asciidoc | 33 +++++++++++++ .../gremlin/process/traversal/Operator.java | 4 +- .../traversal/dsl/graph/GraphTraversal.java | 8 ++-- .../gremlin/process/traversal/dsl/graph/__.java | 8 ++-- .../traversal/step/map/MaxGlobalStep.java | 2 +- .../traversal/step/map/MaxLocalStep.java | 4 +- .../traversal/step/map/MinGlobalStep.java | 2 +- .../traversal/step/map/MinLocalStep.java | 4 +- .../tinkerpop/gremlin/util/NumberHelper.java | 26 ++++++++++ .../traversal/OperatorExceptionTest.java | 10 ---- .../gremlin/process/traversal/OperatorTest.java | 7 ++- gremlin-test/features/map/Max.feature | 22 +++++++++ gremlin-test/features/map/Min.feature | 22 +++++++++ .../process/traversal/step/map/MaxTest.java | 42 +++++++++++++--- .../process/traversal/step/map/MinTest.java | 50 ++++++++++++++++---- .../SparkStarBarrierInterceptor.java | 4 +- 18 files changed, 213 insertions(+), 52 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index fcc96b2..7e029ad 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -25,6 +25,7 @@ NEED AND IMAGE This release also includes changes from <<release-3-3-2, 3.3.2>>. +* `min()` and `max()` now support all types implementing `Comparable`. * Change the `toString()` of `Path` to be standardized as other graph elements are. * Fixed a bug in `ReducingBarrierStep`, that returned the provided seed value despite no elements being available. * Changed the order of `select()` scopes. The order is now: maps, side-effects, paths. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/docs/src/reference/the-traversal.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index 1bba5ac..ea2d09a 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -1471,16 +1471,17 @@ link:++http://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/grem [[max-step]] === Max Step -The `max()`-step (*map*) operates on a stream of numbers and determines which is the largest number in the stream. +The `max()`-step (*map*) operates on a stream of comparable objects and determines which is the largest object in the stream. [gremlin-groovy,modern] ---- g.V().values('age').max() g.V().repeat(both()).times(3).values('age').max() +g.V().values('name').max() ---- IMPORTANT: `max(local)` determines the max of the current, local object (not the objects in the traversal stream). -This works for `Collection` and `Number`-type objects. For any other object, a max of `Double.NaN` is returned. +This works for `Collection` and `Comparable`-type objects. *Additional References* @@ -1504,7 +1505,7 @@ g.V().repeat(both()).times(3).values('age').dedup().mean() thus altering the average. IMPORTANT: `mean(local)` determines the mean of the current, local object (not the objects in the traversal stream). -This works for `Collection` and `Number`-type objects. For any other object, a mean of `Double.NaN` is returned. +This works for `Collection` and `Number`-type objects. *Additional References* @@ -1515,16 +1516,17 @@ link:++http://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/grem [[min-step]] === Min Step -The `min()`-step (*map*) operates on a stream of numbers and determines which is the smallest number in the stream. +The `min()`-step (*map*) operates on a stream of comparable objects and determines which is the smallest object in the stream. [gremlin-groovy,modern] ---- g.V().values('age').min() g.V().repeat(both()).times(3).values('age').min() +g.V().values('name').min() ---- IMPORTANT: `min(local)` determines the min of the current, local object (not the objects in the traversal stream). -This works for `Collection` and `Number`-type objects. For any other object, a min of `Double.NaN` is returned. +This works for `Collection` and `Comparable`-type objects. *Additional References* @@ -2537,7 +2539,7 @@ link:++http://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/grem [[sum-step]] === Sum Step -The `sum()`-step (*map*) operates on a stream of numbers and sums the numbers together to yield a double. Note that +The `sum()`-step (*map*) operates on a stream of numbers and sums the numbers together to yield a result. Note that the current traverser number is multiplied by the traverser bulk to determine how many such numbers are being represented. @@ -2548,7 +2550,7 @@ g.V().repeat(both()).times(3).values('age').sum() ---- IMPORTANT: `sum(local)` determines the sum of the current, local object (not the objects in the traversal stream). -This works for `Collection`-type objects. For any other object, a sum of `Double.NaN` is returned. +This works for `Collection`-type objects. *Additional References* http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/docs/src/upgrade/release-3.4.x.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/upgrade/release-3.4.x.asciidoc b/docs/src/upgrade/release-3.4.x.asciidoc index 713916a..57a6fdb 100644 --- a/docs/src/upgrade/release-3.4.x.asciidoc +++ b/docs/src/upgrade/release-3.4.x.asciidoc @@ -37,6 +37,39 @@ needs. See: link:https://issues.apache.org/jira/browse/TINKERPOP-1930[TINKERPOP-1930] +==== Improvements in `min()` and `max()` + +Previously `min()` and `max()` were only working for numeric values. This has been changed and these steps can now operate over any `Comparable` value. The common workaround was the combination +of `order().by()` and `limit()` as shown here: + +[source,groovy] +---- +gremlin> g.V().values('name').order().by().limit(1) // workaround for min() +==>josh +gremlin> g.V().values('name').order().by(decr).limit(1) // workaround for max() +==>vadas +---- + +Any attempt to use `min()` or `max()` on non-numeric values lead to an exception: + +[source,groovy] +---- +gremlin> g.V().values('name').min() +java.lang.String cannot be cast to java.lang.Number +Type ':help' or ':h' for help. +Display stack trace? [yN] +---- + +With the changes in this release these kind of queries became a lot easier: + +[source,groovy] +---- +gremlin> g.V().values('name').min() +==>josh +gremlin> g.V().values('name').max() +==>vadas +---- + ==== Modifications to reducing barrier steps The behavior of `min()`, `max()`, `mean()` and `sum()` has been modified to return no result if there's no input. Previously these steps yielded the internal seed value: http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java index 1f63e14..1f04ca6 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Operator.java @@ -51,12 +51,12 @@ public enum Operator implements BinaryOperator<Object> { }, min { public Object apply(final Object a, final Object b) { - return NumberHelper.min((Number) a, (Number) b); + return NumberHelper.min((Comparable) a, (Comparable) b); } }, max { public Object apply(final Object a, final Object b) { - return NumberHelper.max((Number) a, (Number) b); + return NumberHelper.max((Comparable) a, (Comparable) b); } }, assign { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java index a10ddb2..8722886 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java @@ -845,7 +845,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { * @see <a href="http://tinkerpop.apache.org/docs/${project.version}/reference/#max-step" target="_blank">Reference Documentation - Max Step</a> * @since 3.0.0-incubating */ - public default <E2 extends Number> GraphTraversal<S, E2> max() { + public default <E2 extends Comparable> GraphTraversal<S, E2> max() { this.asAdmin().getBytecode().addStep(Symbols.max); return this.asAdmin().addStep(new MaxGlobalStep<>(this.asAdmin())); } @@ -857,7 +857,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { * @see <a href="http://tinkerpop.apache.org/docs/${project.version}/reference/#max-step" target="_blank">Reference Documentation - Max Step</a> * @since 3.0.0-incubating */ - public default <E2 extends Number> GraphTraversal<S, E2> max(final Scope scope) { + public default <E2 extends Comparable> GraphTraversal<S, E2> max(final Scope scope) { this.asAdmin().getBytecode().addStep(Symbols.max, scope); return this.asAdmin().addStep(scope.equals(Scope.global) ? new MaxGlobalStep<>(this.asAdmin()) : new MaxLocalStep<>(this.asAdmin())); } @@ -869,7 +869,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { * @see <a href="http://tinkerpop.apache.org/docs/${project.version}/reference/#min-step" target="_blank">Reference Documentation - Min Step</a> * @since 3.0.0-incubating */ - public default <E2 extends Number> GraphTraversal<S, E2> min() { + public default <E2 extends Comparable> GraphTraversal<S, E2> min() { this.asAdmin().getBytecode().addStep(Symbols.min); return this.asAdmin().addStep(new MinGlobalStep<>(this.asAdmin())); } @@ -881,7 +881,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { * @see <a href="http://tinkerpop.apache.org/docs/${project.version}/reference/#min-step" target="_blank">Reference Documentation - Min Step</a> * @since 3.0.0-incubating */ - public default <E2 extends Number> GraphTraversal<S, E2> min(final Scope scope) { + public default <E2 extends Comparable> GraphTraversal<S, E2> min(final Scope scope) { this.asAdmin().getBytecode().addStep(Symbols.min, scope); return this.asAdmin().addStep(scope.equals(Scope.global) ? new MinGlobalStep<E2>(this.asAdmin()) : new MinLocalStep<>(this.asAdmin())); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java index 39e5258..dce5497 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java @@ -403,28 +403,28 @@ public class __ { /** * @see GraphTraversal#min() */ - public static <A, B extends Number> GraphTraversal<A, B> min() { + public static <A, B extends Comparable> GraphTraversal<A, B> min() { return __.<A>start().min(); } /** * @see GraphTraversal#min(Scope) */ - public static <A, B extends Number> GraphTraversal<A, B> min(final Scope scope) { + public static <A, B extends Comparable> GraphTraversal<A, B> min(final Scope scope) { return __.<A>start().min(scope); } /** * @see GraphTraversal#max() */ - public static <A, B extends Number> GraphTraversal<A, B> max() { + public static <A, B extends Comparable> GraphTraversal<A, B> max() { return __.<A>start().max(); } /** * @see GraphTraversal#max(Scope) */ - public static <A, B extends Number> GraphTraversal<A, B> max(final Scope scope) { + public static <A, B extends Comparable> GraphTraversal<A, B> max(final Scope scope) { return __.<A>start().max(scope); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/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 8cb798c..2873f55 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 @@ -32,7 +32,7 @@ import java.util.function.BinaryOperator; /** * @author Marko A. Rodriguez (http://markorodriguez.com) */ -public final class MaxGlobalStep<S extends Number> extends ReducingBarrierStep<S, S> { +public final class MaxGlobalStep<S extends Comparable> extends ReducingBarrierStep<S, S> { public MaxGlobalStep(final Traversal.Admin traversal) { super(traversal); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxLocalStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxLocalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxLocalStep.java index 909a4c7..3ad326f 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxLocalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxLocalStep.java @@ -33,7 +33,7 @@ import static org.apache.tinkerpop.gremlin.util.NumberHelper.max; * @author Marko A. Rodriguez (http://markorodriguez.com) * @author Daniel Kuppitz (http://gremlin.guru) */ -public final class MaxLocalStep<E extends Number, S extends Iterable<E>> extends MapStep<S, E> { +public final class MaxLocalStep<E extends Comparable, S extends Iterable<E>> extends MapStep<S, E> { public MaxLocalStep(final Traversal.Admin traversal) { super(traversal); @@ -43,7 +43,7 @@ public final class MaxLocalStep<E extends Number, S extends Iterable<E>> extends protected E map(final Traverser.Admin<S> traverser) { final Iterator<E> iterator = traverser.get().iterator(); if (iterator.hasNext()) { - Number result = iterator.next(); + Comparable result = iterator.next(); while (iterator.hasNext()) { result = max(iterator.next(), result); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/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 e476f5c..781e93d 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 @@ -32,7 +32,7 @@ import java.util.function.BinaryOperator; /** * @author Marko A. Rodriguez (http://markorodriguez.com) */ -public final class MinGlobalStep<S extends Number> extends ReducingBarrierStep<S, S> { +public final class MinGlobalStep<S extends Comparable> extends ReducingBarrierStep<S, S> { public MinGlobalStep(final Traversal.Admin traversal) { super(traversal); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinLocalStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinLocalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinLocalStep.java index 64c89e3..4139a7d 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinLocalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MinLocalStep.java @@ -33,7 +33,7 @@ import static org.apache.tinkerpop.gremlin.util.NumberHelper.min; * @author Marko A. Rodriguez (http://markorodriguez.com) * @author Daniel Kuppitz (http://gremlin.guru) */ -public final class MinLocalStep<E extends Number, S extends Iterable<E>> extends MapStep<S, E> { +public final class MinLocalStep<E extends Comparable, S extends Iterable<E>> extends MapStep<S, E> { public MinLocalStep(final Traversal.Admin traversal) { super(traversal); @@ -43,7 +43,7 @@ public final class MinLocalStep<E extends Number, S extends Iterable<E>> extends protected E map(final Traverser.Admin<S> traverser) { final Iterator<E> iterator = traverser.get().iterator(); if (iterator.hasNext()) { - Number result = iterator.next(); + Comparable result = iterator.next(); while (iterator.hasNext()) { result = min(iterator.next(), result); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java index efd446b..aaf066a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java @@ -345,11 +345,33 @@ public final class NumberHelper { return getHelper(clazz).min.apply(a, b); } + public static Comparable min(final Comparable a, final Comparable b) { + if (a instanceof Number && b instanceof Number) { + final Number an = (Number) a, bn = (Number) b; + final Class<? extends Number> clazz = getHighestCommonNumberClass(an, bn); + return (Comparable) getHelper(clazz).min.apply(an, bn); + } + return isNonValue(a) ? b : + isNonValue(b) ? a : + a.compareTo(b) < 0 ? a : b; + } + public static Number max(final Number a, final Number b) { final Class<? extends Number> clazz = getHighestCommonNumberClass(a, b); return getHelper(clazz).max.apply(a, b); } + public static Comparable max(final Comparable a, final Comparable b) { + if (a instanceof Number && b instanceof Number) { + final Number an = (Number) a, bn = (Number) b; + final Class<? extends Number> clazz = getHighestCommonNumberClass(an, bn); + return (Comparable) getHelper(clazz).max.apply(an, bn); + } + return isNonValue(a) ? b : + isNonValue(b) ? a : + a.compareTo(b) > 0 ? 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); @@ -415,4 +437,8 @@ public final class NumberHelper { private static boolean isNumber(final Number number) { return number != null && !number.equals(Double.NaN); } + + private static boolean isNonValue(final Object value) { + return value instanceof Double && !isNumber((Double) value); + } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorExceptionTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorExceptionTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorExceptionTest.java index 9aa1339..513d04f 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorExceptionTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorExceptionTest.java @@ -31,16 +31,6 @@ public class OperatorExceptionTest { } @Test(expected = ClassCastException.class) - public void shouldThrowIfValueToMaxIsNotNumeric() { - Operator.max.apply("1", "1"); - } - - @Test(expected = ClassCastException.class) - public void shouldThrowIfValueToMinIsNotNumeric() { - Operator.min.apply("1", "1"); - } - - @Test(expected = ClassCastException.class) public void shouldThrowIfValueToMinusIsNotNumeric() { Operator.minus.apply("1", "1"); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorTest.java index 38f7742..e498cae 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/OperatorTest.java @@ -38,7 +38,7 @@ public class OperatorTest { /** * Required to verify that Operator can handle Number type, that it doesn't know explicitly. */ - static class CustomNumber extends Number { + static class CustomNumber extends Number implements Comparable<CustomNumber> { public final static CustomNumber ONE = new CustomNumber(1); public final static CustomNumber TEN = new CustomNumber(10); @@ -68,6 +68,11 @@ public class OperatorTest { public double doubleValue() { return n; } + + @Override + public int compareTo(final CustomNumber anotherCustomNumber) { + return Integer.compare(n, anotherCustomNumber.n); + } } @Parameterized.Parameters(name = "{0}({1},{2}) = {3}") http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-test/features/map/Max.feature ---------------------------------------------------------------------- diff --git a/gremlin-test/features/map/Max.feature b/gremlin-test/features/map/Max.feature index 7139f65..44d9640 100644 --- a/gremlin-test/features/map/Max.feature +++ b/gremlin-test/features/map/Max.feature @@ -37,6 +37,17 @@ Feature: Step - max() When iterated to list Then the result should be empty + Scenario: g_V_name_max + Given the modern graph + And the traversal of + """ + g.V().values("name").max() + """ + When iterated to list + Then the result should be unordered + | result | + | vadas | + Scenario: g_V_age_fold_maxXlocalX Given the modern graph And the traversal of @@ -57,6 +68,17 @@ Feature: Step - max() When iterated to list Then the result should be empty + Scenario: g_V_name_fold_maxXlocalX + Given the modern graph + And the traversal of + """ + g.V().values("name").fold().max(Scope.local) + """ + When iterated to list + Then the result should be unordered + | result | + | vadas | + Scenario: g_V_repeatXbothX_timesX5X_age_max Given the modern graph And the traversal of http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-test/features/map/Min.feature ---------------------------------------------------------------------- diff --git a/gremlin-test/features/map/Min.feature b/gremlin-test/features/map/Min.feature index 6e3fb5d..d15eff7 100644 --- a/gremlin-test/features/map/Min.feature +++ b/gremlin-test/features/map/Min.feature @@ -37,6 +37,17 @@ Feature: Step - min() When iterated to list Then the result should be empty + Scenario: g_V_name_min + Given the modern graph + And the traversal of + """ + g.V().values("name").min() + """ + When iterated to list + Then the result should be unordered + | result | + | josh | + Scenario: g_V_age_fold_minXlocalX Given the modern graph And the traversal of @@ -57,6 +68,17 @@ Feature: Step - min() When iterated to list Then the result should be empty + Scenario: g_V_name_fold_minXlocalX + Given the modern graph + And the traversal of + """ + g.V().values("name").fold().min(Scope.local) + """ + When iterated to list + Then the result should be unordered + | result | + | josh | + Scenario: g_V_repeatXbothX_timesX5X_age_min Given the modern graph And the traversal of http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxTest.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxTest.java index f13cdb5..22f68de 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MaxTest.java @@ -44,9 +44,13 @@ public abstract class MaxTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, Integer> get_g_V_age_fold_maxXlocalX(); - public abstract Traversal<Vertex, Number> get_g_V_foo_max(); + public abstract Traversal<Vertex, Comparable> get_g_V_foo_max(); - public abstract Traversal<Vertex, Number> get_g_V_foo_fold_maxXlocalX(); + public abstract Traversal<Vertex, Comparable> get_g_V_foo_fold_maxXlocalX(); + + public abstract Traversal<Vertex, String> get_g_V_name_max(); + + public abstract Traversal<Vertex, String> get_g_V_name_fold_maxXlocalX(); public abstract Traversal<Vertex, Integer> get_g_V_repeatXbothX_timesX5X_age_max(); @@ -71,7 +75,7 @@ public abstract class MaxTest extends AbstractGremlinProcessTest { @Test @LoadGraphWith(MODERN) public void g_V_foo_max() { - final Traversal<Vertex, Number> traversal = get_g_V_foo_max(); + final Traversal<Vertex, Comparable> traversal = get_g_V_foo_max(); printTraversalForm(traversal); assertFalse(traversal.hasNext()); } @@ -79,13 +83,29 @@ public abstract class MaxTest extends AbstractGremlinProcessTest { @Test @LoadGraphWith(MODERN) public void g_V_foo_fold_maxXlocalX() { - final Traversal<Vertex, Number> traversal = get_g_V_foo_fold_maxXlocalX(); + final Traversal<Vertex, Comparable> traversal = get_g_V_foo_fold_maxXlocalX(); printTraversalForm(traversal); assertFalse(traversal.hasNext()); } @Test @LoadGraphWith(MODERN) + public void g_V_name_max() { + final Traversal<Vertex, String> traversal = get_g_V_name_max(); + printTraversalForm(traversal); + checkResults(Arrays.asList("vadas"), traversal); + } + + @Test + @LoadGraphWith(MODERN) + public void g_V_name_fold_maxXlocalX() { + final Traversal<Vertex, String> traversal = get_g_V_name_fold_maxXlocalX(); + printTraversalForm(traversal); + checkResults(Arrays.asList("vadas"), traversal); + } + + @Test + @LoadGraphWith(MODERN) public void g_V_repeatXbothX_timesX5X_age_max() { final Traversal<Vertex, Integer> traversal = get_g_V_repeatXbothX_timesX5X_age_max(); printTraversalForm(traversal); @@ -118,16 +138,26 @@ public abstract class MaxTest extends AbstractGremlinProcessTest { } @Override - public Traversal<Vertex, Number> get_g_V_foo_max() { + public Traversal<Vertex, Comparable> get_g_V_foo_max() { return g.V().values("foo").max(); } @Override - public Traversal<Vertex, Number> get_g_V_foo_fold_maxXlocalX() { + public Traversal<Vertex, Comparable> get_g_V_foo_fold_maxXlocalX() { return g.V().values("foo").fold().max(Scope.local); } @Override + public Traversal<Vertex, String> get_g_V_name_max() { + return g.V().values("name").max(); + } + + @Override + public Traversal<Vertex, String> get_g_V_name_fold_maxXlocalX() { + return g.V().values("name").fold().max(Scope.local); + } + + @Override public Traversal<Vertex, Integer> get_g_V_repeatXbothX_timesX5X_age_max() { return g.V().repeat(both()).times(5).values("age").max(); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/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 10f6bc8..06093cb 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 @@ -46,15 +46,19 @@ public abstract class MinTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, Integer> get_g_V_age_fold_minXlocalX(); - public abstract Traversal<Vertex, Number> get_g_V_foo_min(); + public abstract Traversal<Vertex, Comparable> get_g_V_foo_min(); - public abstract Traversal<Vertex, Number> get_g_V_foo_fold_minXlocalX(); + public abstract Traversal<Vertex, Comparable> get_g_V_foo_fold_minXlocalX(); + + public abstract Traversal<Vertex, String> get_g_V_name_min(); + + public abstract Traversal<Vertex, String> get_g_V_name_fold_minXlocalX(); public abstract Traversal<Vertex, Integer> get_g_V_repeatXbothX_timesX5X_age_min(); 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(); + public abstract Traversal<Vertex, Comparable> get_g_V_foo_injectX9999999999X_min(); @Test @LoadGraphWith(MODERN) @@ -75,7 +79,7 @@ public abstract class MinTest extends AbstractGremlinProcessTest { @Test @LoadGraphWith(MODERN) public void g_V_foo_min() { - final Traversal<Vertex, Number> traversal = get_g_V_foo_min(); + final Traversal<Vertex, Comparable> traversal = get_g_V_foo_min(); printTraversalForm(traversal); assertFalse(traversal.hasNext()); } @@ -83,13 +87,29 @@ public abstract class MinTest extends AbstractGremlinProcessTest { @Test @LoadGraphWith(MODERN) public void g_V_foo_fold_minXlocalX() { - final Traversal<Vertex, Number> traversal = get_g_V_foo_fold_minXlocalX(); + final Traversal<Vertex, Comparable> traversal = get_g_V_foo_fold_minXlocalX(); printTraversalForm(traversal); assertFalse(traversal.hasNext()); } @Test @LoadGraphWith(MODERN) + public void g_V_name_min() { + final Traversal<Vertex, String> traversal = get_g_V_name_min(); + printTraversalForm(traversal); + checkResults(Arrays.asList("josh"), traversal); + } + + @Test + @LoadGraphWith(MODERN) + public void g_V_name_fold_minXlocalX() { + final Traversal<Vertex, String> traversal = get_g_V_name_fold_minXlocalX(); + printTraversalForm(traversal); + checkResults(Arrays.asList("josh"), traversal); + } + + @Test + @LoadGraphWith(MODERN) public void g_V_repeatXbothX_timesX5X_age_min() { final Traversal<Vertex, Integer> traversal = get_g_V_repeatXbothX_timesX5X_age_min(); printTraversalForm(traversal); @@ -112,10 +132,10 @@ public abstract class MinTest extends AbstractGremlinProcessTest { @Test @LoadGraphWith(MODERN) public void g_V_foo_injectX9999999999X_min() { - final Traversal<Vertex, Number> traversal = get_g_V_foo_injectX9999999999X_min(); + final Traversal<Vertex, Comparable> traversal = get_g_V_foo_injectX9999999999X_min(); printTraversalForm(traversal); assertTrue(traversal.hasNext()); - assertEquals(9999999999L, traversal.next().longValue()); + assertEquals(9999999999L, traversal.next()); assertFalse(traversal.hasNext()); } @@ -132,16 +152,26 @@ public abstract class MinTest extends AbstractGremlinProcessTest { } @Override - public Traversal<Vertex, Number> get_g_V_foo_min() { + public Traversal<Vertex, Comparable> get_g_V_foo_min() { return g.V().values("foo").min(); } @Override - public Traversal<Vertex, Number> get_g_V_foo_fold_minXlocalX() { + public Traversal<Vertex, Comparable> get_g_V_foo_fold_minXlocalX() { return g.V().values("foo").fold().min(Scope.local); } @Override + public Traversal<Vertex, String> get_g_V_name_min() { + return g.V().values("name").min(); + } + + @Override + public Traversal<Vertex, String> get_g_V_name_fold_minXlocalX() { + return g.V().values("name").fold().min(Scope.local); + } + + @Override public Traversal<Vertex, Integer> get_g_V_repeatXbothX_timesX5X_age_min() { return g.V().repeat(both()).times(5).values("age").min(); } @@ -152,7 +182,7 @@ public abstract class MinTest extends AbstractGremlinProcessTest { } @Override - public Traversal<Vertex, Number> get_g_V_foo_injectX9999999999X_min() { + public Traversal<Vertex, Comparable> get_g_V_foo_injectX9999999999X_min() { return g.V().values("foo").inject(9999999999L).min(); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a1b0ed6e/spark-gremlin/src/main/java/org/apache/tinkerpop/gremlin/spark/process/computer/traversal/strategy/optimization/interceptor/SparkStarBarrierInterceptor.java ---------------------------------------------------------------------- diff --git a/spark-gremlin/src/main/java/org/apache/tinkerpop/gremlin/spark/process/computer/traversal/strategy/optimization/interceptor/SparkStarBarrierInterceptor.java b/spark-gremlin/src/main/java/org/apache/tinkerpop/gremlin/spark/process/computer/traversal/strategy/optimization/interceptor/SparkStarBarrierInterceptor.java index 6509928..de42525 100644 --- a/spark-gremlin/src/main/java/org/apache/tinkerpop/gremlin/spark/process/computer/traversal/strategy/optimization/interceptor/SparkStarBarrierInterceptor.java +++ b/spark-gremlin/src/main/java/org/apache/tinkerpop/gremlin/spark/process/computer/traversal/strategy/optimization/interceptor/SparkStarBarrierInterceptor.java @@ -110,11 +110,11 @@ public final class SparkStarBarrierInterceptor implements SparkVertexProgramInte .getFinal(); } else if (endStep instanceof MinGlobalStep) { result = nextRDD.isEmpty() ? null : nextRDD - .map(traverser -> (Number) traverser.get()) + .map(traverser -> (Comparable) traverser.get()) .fold(Double.NaN, NumberHelper::min); } else if (endStep instanceof MaxGlobalStep) { result = nextRDD.isEmpty() ? null : nextRDD - .map(traverser -> (Number) traverser.get()) + .map(traverser -> (Comparable) traverser.get()) .fold(Double.NaN, NumberHelper::max); } else if (endStep instanceof FoldStep) { final BinaryOperator biOperator = endStep.getBiOperator();