Repository: tinkerpop Updated Branches: refs/heads/shortest-path-wip [created] c8d75ed3d
TINKERPOP-1958 Fixed a bug in TinkerGraphCountStrategy The strategy did not consider, that certain map steps may not emit an element. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/c374dbe7 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/c374dbe7 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/c374dbe7 Branch: refs/heads/shortest-path-wip Commit: c374dbe7609d9110536270517658b4b3dc27ffd0 Parents: 44e4332 Author: Daniel Kuppitz <[email protected]> Authored: Wed May 9 07:53:24 2018 -0700 Committer: Daniel Kuppitz <[email protected]> Committed: Fri May 25 13:05:00 2018 -0700 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + gremlin-test/features/map/Select.feature | 20 ++++++++++++++++ .../process/traversal/step/map/SelectTest.java | 15 ++++++++++++ .../optimization/TinkerGraphCountStrategy.java | 2 +- .../TinkerGraphCountStrategyTest.java | 25 ++++++++++++-------- 5 files changed, 52 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/c374dbe7/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index b9ef257..30fd966 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -395,6 +395,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Improved performance of `TraversalVertexProgram` and related infrastructure. * Fixed bug in `GroovyTranslator` that didn't properly handle empty `Map` objects. * Added concrete configuration methods to `SparkGraphComputer` to make a more clear API for configuring it. +* Fixed a bug in `TinkerGraphCountStrategy`, which didn't consider that certain map steps may not emit an element. [[release-3-2-9]] === TinkerPop 3.2.9 (Release Date: May 8, 2018) http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/c374dbe7/gremlin-test/features/map/Select.feature ---------------------------------------------------------------------- diff --git a/gremlin-test/features/map/Select.feature b/gremlin-test/features/map/Select.feature index a5c28e5..12a406e 100644 --- a/gremlin-test/features/map/Select.feature +++ b/gremlin-test/features/map/Select.feature @@ -575,3 +575,23 @@ Feature: Step - select() | result | | l[v[marko],v[vadas]] | | l[v[marko],v[josh]] | + + Scenario: g_V_selectXaX + Given the modern graph + And the traversal of + """ + g.V().select("a") + """ + When iterated to list + Then the result should be empty + + Scenario: g_V_selectXaX_count + Given the modern graph + And the traversal of + """ + g.V().select("a").count() + """ + When iterated to list + Then the result should be unordered + | result | + | d[0].l | http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/c374dbe7/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java index 9456dec..c5651eb 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java @@ -91,6 +91,8 @@ public abstract class SelectTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, List<Vertex>> get_g_V_asXaX_outXknowsX_asXaX_selectXall_constantXaXX(); + public abstract Traversal<Vertex, Long> get_g_V_selectXaX_count(); + // below are original back()-tests public abstract Traversal<Vertex, Vertex> get_g_VX1X_asXhereX_out_selectXhereX(final Object v1Id); @@ -386,6 +388,14 @@ public abstract class SelectTest extends AbstractGremlinProcessTest { checkResults(Arrays.asList(Arrays.asList(marko, vadas), Arrays.asList(marko, josh)), traversal); } + @Test + @LoadGraphWith(MODERN) + public void g_V_selectXaX_count() { + final Traversal<Vertex, Long> traversal = get_g_V_selectXaX_count(); + printTraversalForm(traversal); + assertEquals(0L, traversal.next().longValue()); + } + // below are original back()-tests @Test @@ -788,6 +798,11 @@ public abstract class SelectTest extends AbstractGremlinProcessTest { return g.V().as("a").out("knows").as("a").select(Pop.all, (Traversal) __.constant("a")); } + @Override + public Traversal<Vertex, Long> get_g_V_selectXaX_count() { + return g.V().select("a").count(); + } + // below are original back()-tests @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/c374dbe7/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategy.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategy.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategy.java index 50e5c18..55e6b55 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategy.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategy.java @@ -71,7 +71,7 @@ public final class TinkerGraphCountStrategy extends AbstractTraversalStrategy<Tr return; for (int i = 1; i < steps.size() - 1; i++) { final Step current = steps.get(i); - if (!(current instanceof MapStep || + if (!(//current instanceof MapStep || // MapSteps will not necessarily emit an element as demonstrated in https://issues.apache.org/jira/browse/TINKERPOP-1958 current instanceof IdentityStep || current instanceof NoOpBarrierStep || current instanceof CollectingBarrierStep) || http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/c374dbe7/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategyTest.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategyTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategyTest.java index 0db378b..ec9bd93 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategyTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphCountStrategyTest.java @@ -64,6 +64,11 @@ public class TinkerGraphCountStrategyTest { for (final TraversalStrategy strategy : this.otherStrategies) { strategies.addStrategies(strategy); } + if (this.optimized == null) { + this.optimized = this.original.asAdmin().clone(); + this.optimized.asAdmin().setStrategies(strategies); + this.optimized.asAdmin().applyStrategies(); + } this.original.asAdmin().setStrategies(strategies); this.original.asAdmin().applyStrategies(); assertEquals(this.optimized, this.original); @@ -81,17 +86,17 @@ public class TinkerGraphCountStrategyTest { {__.V().count(), countStep(Vertex.class), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, {__.V().as("a").count(), countStep(Vertex.class), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, {__.V().count().as("a"), countStep(Vertex.class), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, - {__.V().map(out()).count().as("a"), countStep(Vertex.class), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, - {__.V().map(out()).identity().count().as("a"), countStep(Vertex.class), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, - {__.V().map(out().groupCount()).identity().count().as("a"), countStep(Vertex.class), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, - {__.V().label().map(s -> s.get().length()).count(), countStep(Vertex.class), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, - {__.V().as("a").map(select("a")).count(), countStep(Vertex.class),TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + {__.V().map(out()).count().as("a"), null, TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + {__.V().map(out()).identity().count().as("a"), null, TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + {__.V().map(out().groupCount()).identity().count().as("a"), null, TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + {__.V().label().map(s -> s.get().length()).count(), null, TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + {__.V().as("a").map(select("a")).count(), null, TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, // - {__.V(), __.V(), Collections.emptyList()}, - {__.V().out().count(), __.V().out().count(), Collections.emptyList()}, - {__.V(1).count(), __.V(1).count(), Collections.emptyList()}, - {__.count(), __.count(), Collections.emptyList()}, - {__.V().map(out().groupCount("m")).identity().count().as("a"), __.V().map(out().groupCount("m")).identity().count().as("a"), Collections.emptyList()}, + {__.V(), null, Collections.emptyList()}, + {__.V().out().count(), null, Collections.emptyList()}, + {__.V(1).count(), null, Collections.emptyList()}, + {__.count(), null, Collections.emptyList()}, + {__.V().map(out().groupCount("m")).identity().count().as("a"), null, Collections.emptyList()}, }); } }
