Repository: tinkerpop Updated Branches: refs/heads/master 227b85cb3 -> 02cffb58e
LazyBarrierStrategy caused Neo4jGraphStepStrategyTest to fail. I fixed up Neo4jGraphStepStrategyTest to be like TinkerGraphStepStrategyTest. Moreover, both Neo4jGraphStepStrategy and TinkerGraphStepStrategy are smart about NoOpBarrierSteps. Given that we use these everywhere now -- PathRetractionStratey, RepeatUnrollStrategy, LazyBarrierStrategy, barrier() -- provider strategies should look out for them accordingly. I updated the upgrade docs with this information and posted to gremlin-users/dev so providers are aware. Also removed a print statement in a Gremlin-Python test and removed a stream()-usage in TraverserSet. CTR as this is related to the LazyBarrierStrategy ticket that was already merged. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/02cffb58 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/02cffb58 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/02cffb58 Branch: refs/heads/master Commit: 02cffb58e9ae0974631add15540378d3f322525f Parents: 227b85c Author: Marko A. Rodriguez <okramma...@gmail.com> Authored: Thu Oct 13 08:32:32 2016 -0600 Committer: Marko A. Rodriguez <okramma...@gmail.com> Committed: Thu Oct 13 08:32:32 2016 -0600 ---------------------------------------------------------------------- docs/src/reference/the-traversal.asciidoc | 22 +-- .../upgrade/release-3.2.x-incubating.asciidoc | 32 +++-- .../traversal/traverser/util/TraverserSet.java | 6 +- .../driver/test_driver_remote_connection.py | 5 +- .../optimization/Neo4jGraphStepStrategy.java | 15 +- .../traversal/strategy/Neo4jStrategySuite.java | 44 ------ .../traversal/strategy/Neo4jStrategyTest.java | 32 ----- .../Neo4jGraphStepStrategyTest.java | 139 +++++++++++++------ .../optimization/TinkerGraphStepStrategy.java | 15 +- .../TinkerGraphStepStrategyTest.java | 18 ++- 10 files changed, 175 insertions(+), 153 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/docs/src/reference/the-traversal.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index 336068f..c657d01 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -2448,13 +2448,15 @@ public final class TinkerGraphStepStrategy extends AbstractTraversalStrategy<Tra final TinkerGraphStep<?, ?> tinkerGraphStep = new TinkerGraphStep<>(originalGraphStep); TraversalHelper.replaceStep(originalGraphStep, tinkerGraphStep, traversal); Step<?, ?> currentStep = tinkerGraphStep.getNextStep(); - while (currentStep instanceof HasStep) { - for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) { - if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer)) - tinkerGraphStep.addHasContainer(hasContainer); + while (currentStep instanceof HasStep || currentStep instanceof NoOpBarrierStep) { + if (currentStep instanceof HasStep) { + for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) { + if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer)) + tinkerGraphStep.addHasContainer(hasContainer); + } + TraversalHelper.copyLabels(currentStep, currentStep.getPreviousStep(), false); + traversal.removeStep(currentStep); } - TraversalHelper.copyLabels(currentStep, tinkerGraphStep, false); - traversal.removeStep(currentStep); currentStep = currentStep.getNextStep(); } } @@ -2467,11 +2469,9 @@ public final class TinkerGraphStepStrategy extends AbstractTraversalStrategy<Tra ---- The traversal is redefined by simply taking a chain of `has()`-steps after `g.V()` (`TinkerGraphStep`) and providing -their `HasContainers` to `TinkerGraphStep`. Then its up to `TinkerGraphStep` to determine if an appropriate index exists. In the code -below, review the `vertices()` method and note how if an index exists, for a particular `HasContainer`, then that -index is first queried before the remaining `HasContainer` filters are serially applied. Given that the strategy -uses non-TinkerPop3 provided steps, it should go into the `ProviderOptimizationStrategy` category to ensure the -added step does not interfere with the assumptions of the `OptimizationStrategy` strategies. +their `HasContainers` to `TinkerGraphStep`. Then its up to `TinkerGraphStep` to determine if an appropriate index exists. +Given that the strategy uses non-TinkerPop3 provided steps, it should go into the `ProviderOptimizationStrategy` category +to ensure the added step does not interfere with the assumptions of the `OptimizationStrategy` strategies. [gremlin-groovy,modern] ---- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/docs/src/upgrade/release-3.2.x-incubating.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc index c21f1a5..4808473 100644 --- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc +++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc @@ -167,14 +167,30 @@ Upgrading for Providers Graph System Providers ^^^^^^^^^^^^^^^^^^^^^^ +Default LazyBarrierStrategy ++++++++++++++++++++++++++++ + +`LazyBarrierStrategy` has been included as a default strategy. `LazyBarrierStrategy` walks a traversal and looks for +"flatMaps" (`out()`, `in()`, `both()`, `values()`, `V()`, etc.) and adds "lazy barriers" to dam up the stream so to +increase the probability of bulking the traversers. One of the side-effects is that: + +[source,java] +g.V().out().V().has(a) + +is compiled to: + +[source,java] +g.V().out().barrier().V().barrier().has(a) + +Given that `LazyBarrierStrategy` is an `OptimizationStrategy`, it comes before `ProviderOptimizationStrategies`. +Thus, if the provider's `XXXGraphStepStrategy` simply walks from the second `V()` looking for `has()`-only, it will not +be able to pull in the `has()` cause the `barrier()` blocks it. Please see the updates to `TinkerGraphStepStrategy` and +how it acknowledges `NoOpBarrierSteps` (i.e. `barrier()``) skipping over them and âleftâ-propagating labels to the +previous step. + Configurable Strategies +++++++++++++++++++++++ -`TraversalSource.withStrategies()` and `TraversalSource.withoutStrategies()` use Java objects. In order to make strategy -manipulation possible from Gremlin language variants like Gremlin-Python, it is important to support non-JVM-based versions -of these methods. As such, `TraversalSource.withStrategies(Map<String,Object>...)` and `TraversalSource.withoutStrategies(String...)` -were added. - If the provider has non-configurable `TraversalStrategy` classes, those classes should expose a static `instance()`-method. This is typical and thus, backwards compatible. However, if the provider has a `TraversalStrategy` that can be configured (e.g. via a `Builder`), then it should expose a static `create(Configuration)`-method, where the keys of the configuration @@ -184,9 +200,7 @@ a `SubgraphStrategy`, it does the following: [source,python] ---- g = Graph().traversal().withRemote(connection). - withStrategy({'strategy':'org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy', - 'vertices': __.hasLabel('person'), - 'edges': __.has('weight',gt(0.5))}) + withStrategies(SubgraphStrategy(vertices=__.hasLabel('person'),edges=__.has('weight',gt(0.5)))) --- The `SubgraphStrategy.create(Configuration)`-method is defined as: @@ -232,7 +246,7 @@ The default implementation of `TraversalStrategy.getConfiguration()` is defined [source,java] ---- public default Configuration getConfiguration() { - return new MapConfiguration(Collections.singletonMap(STRATEGY, this.getClass().getCanonicalName())); + return new BaseConfiguration(); } ---- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/util/TraverserSet.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/util/TraverserSet.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/util/TraverserSet.java index d6461ce..3e3d5ea 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/util/TraverserSet.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/traverser/util/TraverserSet.java @@ -65,7 +65,11 @@ public class TraverserSet<S> extends AbstractSet<Traverser.Admin<S>> implements } public long bulkSize() { - return this.map.values().stream().map(Traverser::bulk).reduce(0l, (a, b) -> a + b); + long bulk = 0L; + for (final Traverser.Admin<S> traverser : this.map.values()) { + bulk = bulk + traverser.bulk(); + } + return bulk; } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/gremlin-python/src/main/jython/tests/driver/test_driver_remote_connection.py ---------------------------------------------------------------------- diff --git a/gremlin-python/src/main/jython/tests/driver/test_driver_remote_connection.py b/gremlin-python/src/main/jython/tests/driver/test_driver_remote_connection.py index 6e4c0f5..46fb3bc 100644 --- a/gremlin-python/src/main/jython/tests/driver/test_driver_remote_connection.py +++ b/gremlin-python/src/main/jython/tests/driver/test_driver_remote_connection.py @@ -21,7 +21,6 @@ __author__ = 'Marko A. Rodriguez (http://markorodriguez.com)' import unittest from unittest import TestCase -from gremlin_python.structure.io.graphson import GraphSONWriter import pytest @@ -68,6 +67,7 @@ class TestDriverRemoteConnection(TestCase): def test_strategies(self): statics.load_statics(globals()) connection = DriverRemoteConnection('ws://localhost:8182/gremlin', 'g') + # g = Graph().traversal().withRemote(connection). \ withStrategies(TraversalStrategy("SubgraphStrategy", {"vertices": __.hasLabel("person"), @@ -79,7 +79,6 @@ class TestDriverRemoteConnection(TestCase): # g = Graph().traversal().withRemote(connection). \ withStrategies(SubgraphStrategy(vertices=__.hasLabel("person"), edges=__.hasLabel("created"))) - print GraphSONWriter().writeObject(g.bytecode) assert 4 == g.V().count().next() assert 0 == g.E().count().next() assert 1 == g.V().label().dedup().count().next() @@ -91,7 +90,7 @@ class TestDriverRemoteConnection(TestCase): assert 0 == g.E().count().next() assert "person" == g.V().label().next() assert "marko" == g.V().name.next() - + # g = Graph().traversal().withRemote(connection).withComputer() assert 6 == g.V().count().next() assert 6 == g.E().count().next() http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategy.java ---------------------------------------------------------------------- diff --git a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategy.java b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategy.java index 071c248..d610964 100644 --- a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategy.java +++ b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategy.java @@ -25,6 +25,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.NoOpBarrierStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; @@ -46,13 +47,15 @@ public final class Neo4jGraphStepStrategy extends AbstractTraversalStrategy<Trav final Neo4jGraphStep<?, ?> neo4jGraphStep = new Neo4jGraphStep<>(originalGraphStep); TraversalHelper.replaceStep(originalGraphStep, neo4jGraphStep, traversal); Step<?, ?> currentStep = neo4jGraphStep.getNextStep(); - while (currentStep instanceof HasStep) { - for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) { - if (!GraphStep.processHasContainerIds(neo4jGraphStep, hasContainer)) - neo4jGraphStep.addHasContainer(hasContainer); + while (currentStep instanceof HasStep || currentStep instanceof NoOpBarrierStep) { + if (currentStep instanceof HasStep) { + for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) { + if (!GraphStep.processHasContainerIds(neo4jGraphStep, hasContainer)) + neo4jGraphStep.addHasContainer(hasContainer); + } + TraversalHelper.copyLabels(currentStep, currentStep.getPreviousStep(), false); + traversal.removeStep(currentStep); } - TraversalHelper.copyLabels(currentStep, neo4jGraphStep, false); - traversal.removeStep(currentStep); currentStep = currentStep.getNextStep(); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategySuite.java ---------------------------------------------------------------------- diff --git a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategySuite.java b/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategySuite.java deleted file mode 100644 index 0004bc2..0000000 --- a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategySuite.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.tinkerpop.gremlin.neo4j.process.traversal.strategy; - -import org.apache.tinkerpop.gremlin.AbstractGremlinSuite; -import org.apache.tinkerpop.gremlin.neo4j.process.traversal.strategy.optimization.Neo4jGraphStepStrategyTest; -import org.apache.tinkerpop.gremlin.process.traversal.TraversalEngine; -import org.junit.runners.model.InitializationError; -import org.junit.runners.model.RunnerBuilder; - -/** - * @author Daniel Kuppitz (http://gremlin.guru) - */ -public class Neo4jStrategySuite extends AbstractGremlinSuite { - - public Neo4jStrategySuite(final Class<?> klass, final RunnerBuilder builder) throws InitializationError { - - super(klass, builder, - new Class<?>[]{ - Neo4jGraphStepStrategyTest.class - }, new Class<?>[]{ - Neo4jGraphStepStrategyTest.class - }, - false, - TraversalEngine.Type.STANDARD); - } - -} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategyTest.java ---------------------------------------------------------------------- diff --git a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategyTest.java b/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategyTest.java deleted file mode 100644 index 5363e1f..0000000 --- a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/Neo4jStrategyTest.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.tinkerpop.gremlin.neo4j.process.traversal.strategy; - -import org.apache.tinkerpop.gremlin.GraphProviderClass; -import org.apache.tinkerpop.gremlin.neo4j.NoMultiNoMetaNeo4jGraphProvider; -import org.apache.tinkerpop.gremlin.neo4j.structure.Neo4jGraph; -import org.junit.runner.RunWith; - -/** - * @author Daniel Kuppitz (http://gremlin.guru) - */ -@RunWith(Neo4jStrategySuite.class) -@GraphProviderClass(provider = NoMultiNoMetaNeo4jGraphProvider.class, graph = Neo4jGraph.class) -public class Neo4jStrategyTest { -} http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java ---------------------------------------------------------------------- diff --git a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java b/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java index 0611e23..3f8cd02 100644 --- a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java +++ b/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java @@ -20,57 +20,116 @@ package org.apache.tinkerpop.gremlin.neo4j.process.traversal.strategy.optimizati import org.apache.tinkerpop.gremlin.neo4j.AbstractNeo4jGremlinTest; import org.apache.tinkerpop.gremlin.neo4j.process.traversal.step.sideEffect.Neo4jGraphStep; +import org.apache.tinkerpop.gremlin.neo4j.structure.Neo4jGraph; import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies; +import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.DefaultGraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.FilterRankingStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.InlineFilterStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.LazyBarrierStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies; +import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversal; +import org.apache.tinkerpop.gremlin.structure.Vertex; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; + +import static org.apache.tinkerpop.gremlin.process.traversal.P.eq; +import static org.apache.tinkerpop.gremlin.process.traversal.P.gt; +import static org.apache.tinkerpop.gremlin.process.traversal.P.lt; +import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.filter; +import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has; +import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.not; +import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.properties; import static org.junit.Assert.assertEquals; /** + * @author Marko Rodriguez (http://markorodriguez.com) * @author Daniel Kuppitz (http://gremlin.guru) */ -public class Neo4jGraphStepStrategyTest extends AbstractNeo4jGremlinTest { +@RunWith(Parameterized.class) +public class Neo4jGraphStepStrategyTest { + + @Parameterized.Parameter(value = 0) + public Traversal original; + + @Parameterized.Parameter(value = 1) + public Traversal optimized; + + @Parameterized.Parameter(value = 2) + public Collection<TraversalStrategy> otherStrategies; @Test - public void shouldFoldInHasContainers() { - GraphTraversal.Admin traversal = g.V().has("name", "marko").asAdmin(); - assertEquals(2, traversal.getSteps().size()); - assertEquals(HasStep.class, traversal.getEndStep().getClass()); - traversal.applyStrategies(); - assertEquals(1, traversal.getSteps().size()); - assertEquals(Neo4jGraphStep.class, traversal.getStartStep().getClass()); - assertEquals(Neo4jGraphStep.class, traversal.getEndStep().getClass()); - assertEquals(1, ((Neo4jGraphStep) traversal.getStartStep()).getHasContainers().size()); - assertEquals("name", ((Neo4jGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey()); - assertEquals("marko", ((Neo4jGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue()); - //// - traversal = g.V().has("name", "marko").has("age", P.gt(20)).asAdmin(); - traversal.applyStrategies(); - assertEquals(1, traversal.getSteps().size()); - assertEquals(Neo4jGraphStep.class, traversal.getStartStep().getClass()); - assertEquals(2, ((Neo4jGraphStep) traversal.getStartStep()).getHasContainers().size()); - //// - traversal = g.V().has("name", "marko").out().has("name", "daniel").asAdmin(); - traversal.applyStrategies(); - assertEquals(3, traversal.getSteps().size()); - assertEquals(Neo4jGraphStep.class, traversal.getStartStep().getClass()); - assertEquals(1, ((Neo4jGraphStep) traversal.getStartStep()).getHasContainers().size()); - assertEquals("name", ((Neo4jGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey()); - assertEquals("marko", ((Neo4jGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue()); - assertEquals(HasStep.class, traversal.getEndStep().getClass()); - //// - traversal = g.V().has("name", "marko").out().V().has("name", "daniel").asAdmin(); - traversal.applyStrategies(); - assertEquals(3, traversal.getSteps().size()); - assertEquals(Neo4jGraphStep.class, traversal.getStartStep().getClass()); - assertEquals(1, ((Neo4jGraphStep) traversal.getStartStep()).getHasContainers().size()); - assertEquals("name", ((Neo4jGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getKey()); - assertEquals("marko", ((Neo4jGraphStep<?, ?>) traversal.getStartStep()).getHasContainers().get(0).getValue()); - assertEquals(Neo4jGraphStep.class, traversal.getSteps().get(2).getClass()); - assertEquals(1, ((Neo4jGraphStep) traversal.getSteps().get(2)).getHasContainers().size()); - assertEquals("name", ((Neo4jGraphStep<?, ?>) traversal.getSteps().get(2)).getHasContainers().get(0).getKey()); - assertEquals("daniel", ((Neo4jGraphStep<?,?>) traversal.getSteps().get(2)).getHasContainers().get(0).getValue()); - assertEquals(Neo4jGraphStep.class, traversal.getEndStep().getClass()); + public void doTest() { + final TraversalStrategies strategies = new DefaultTraversalStrategies(); + strategies.addStrategies(Neo4jGraphStepStrategy.instance()); + for (final TraversalStrategy strategy : this.otherStrategies) { + strategies.addStrategies(strategy); + } + this.original.asAdmin().setStrategies(strategies); + this.original.asAdmin().applyStrategies(); + assertEquals(this.optimized, this.original); + } + + private static GraphTraversal.Admin<?, ?> g_V(final Object... hasKeyValues) { + final GraphTraversal.Admin<?, ?> traversal = new DefaultGraphTraversal<>(); + final Neo4jGraphStep<Vertex, Vertex> graphStep = new Neo4jGraphStep<>(new GraphStep<>(traversal, Vertex.class, true)); + for (int i = 0; i < hasKeyValues.length; i = i + 2) { + graphStep.addHasContainer(new HasContainer((String) hasKeyValues[i], (P) hasKeyValues[i + 1])); + } + return traversal.addStep(graphStep); + } + + private static GraphStep<?, ?> V(final Object... hasKeyValues) { + final Neo4jGraphStep<Vertex, Vertex> graphStep = new Neo4jGraphStep<>(new GraphStep<>(EmptyTraversal.instance(), Vertex.class, true)); + for (int i = 0; i < hasKeyValues.length; i = i + 2) { + graphStep.addHasContainer(new HasContainer((String) hasKeyValues[i], (P) hasKeyValues[i + 1])); + } + return graphStep; + } + + @Parameterized.Parameters(name = "{0}") + public static Iterable<Object[]> generateTestParameters() { + final int LAZY_SIZE = 2500; + return Arrays.asList(new Object[][]{ + {__.V().out(), g_V().out(), Collections.emptyList()}, + {__.V().has("name", "marko").out(), g_V("name", eq("marko")).out(), Collections.emptyList()}, + {__.V().has("name", "marko").has("age", gt(31).and(lt(10))).out(), + g_V("name", eq("marko"), "age", gt(31), "age", lt(10)).out(), Collections.emptyList()}, + {__.V().has("name", "marko").or(has("age"), has("age", gt(32))).has("lang", "java"), + g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))), Collections.singletonList(FilterRankingStrategy.instance())}, + {__.V().has("name", "marko").as("a").or(has("age"), has("age", gt(32))).has("lang", "java"), + g_V("name", eq("marko")).as("a").or(has("age"), has("age", gt(32))).has("lang", "java"), Collections.emptyList()}, + {__.V().has("name", "marko").as("a").or(has("age"), has("age", gt(32))).has("lang", "java"), + g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))).as("a"), Collections.singletonList(FilterRankingStrategy.instance())}, + {__.V().dedup().has("name", "marko").or(has("age"), has("age", gt(32))).has("lang", "java"), + g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))).dedup(), Collections.singletonList(FilterRankingStrategy.instance())}, + {__.V().as("a").dedup().has("name", "marko").or(has("age"), has("age", gt(32))).has("lang", "java"), + g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))).dedup().as("a"), Collections.singletonList(FilterRankingStrategy.instance())}, + {__.V().as("a").has("name", "marko").as("b").or(has("age"), has("age", gt(32))).has("lang", "java"), + g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))).as("b", "a"), Collections.singletonList(FilterRankingStrategy.instance())}, + {__.V().as("a").dedup().has("name", "marko").or(has("age"), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), + g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(has("age"), has("age", gt(32))).dedup().as("a"), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance())}, + {__.V().as("a").dedup().has("name", "marko").or(has("age", 10), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), + g_V("name", eq("marko"), "age", eq(10).or(gt(32)), "name", eq("bob"), "lang", eq("java")).dedup().as("a"), TraversalStrategies.GlobalCache.getStrategies(Neo4jGraph.class).toList()}, + {__.V().has("name", "marko").or(not(has("age")), has("age", gt(32))).has("name", "bob").has("lang", "java"), + g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(not(filter(properties("age"))), has("age", gt(32))), TraversalStrategies.GlobalCache.getStrategies(Neo4jGraph.class).toList()}, + /////// + {__.V().out().out().V().has("name", "marko").out(), g_V().out().barrier(LAZY_SIZE).out().barrier(LAZY_SIZE).asAdmin().addStep(V("name", eq("marko"))).barrier(LAZY_SIZE).out().barrier(LAZY_SIZE), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance(), LazyBarrierStrategy.instance())}, + {__.V().out().out().V().has("name", "marko").as("a").out(), g_V().out().barrier(LAZY_SIZE).out().barrier(LAZY_SIZE).asAdmin().addStep(V("name", eq("marko"))).barrier(LAZY_SIZE).as("a").out(), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance(), LazyBarrierStrategy.instance())}, + {__.V().out().V().has("age", gt(32)).barrier(10).has("name", "marko").as("a"), g_V().out().barrier(LAZY_SIZE).asAdmin().addStep(V("age", gt(32), "name", eq("marko"))).barrier(LAZY_SIZE).barrier(10).as("a"), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance(), LazyBarrierStrategy.instance())}, + {__.V().out().V().has("age", gt(32)).barrier(10).has("name", "marko").as("a"), g_V().out().barrier(LAZY_SIZE).asAdmin().addStep(V("age", gt(32), "name", eq("marko"))).barrier(LAZY_SIZE).barrier(10).as("a"), TraversalStrategies.GlobalCache.getStrategies(Neo4jGraph.class).toList()}, + }); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java index 98601f6..804eabd 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategy.java @@ -24,6 +24,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.NoOpBarrierStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; @@ -48,13 +49,15 @@ public final class TinkerGraphStepStrategy extends AbstractTraversalStrategy<Tra final TinkerGraphStep<?, ?> tinkerGraphStep = new TinkerGraphStep<>(originalGraphStep); TraversalHelper.replaceStep(originalGraphStep, tinkerGraphStep, traversal); Step<?, ?> currentStep = tinkerGraphStep.getNextStep(); - while (currentStep instanceof HasStep) { - for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) { - if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer)) - tinkerGraphStep.addHasContainer(hasContainer); + while (currentStep instanceof HasStep || currentStep instanceof NoOpBarrierStep) { + if (currentStep instanceof HasStep) { + for (final HasContainer hasContainer : ((HasContainerHolder) currentStep).getHasContainers()) { + if (!GraphStep.processHasContainerIds(tinkerGraphStep, hasContainer)) + tinkerGraphStep.addHasContainer(hasContainer); + } + TraversalHelper.copyLabels(currentStep, currentStep.getPreviousStep(), false); + traversal.removeStep(currentStep); } - TraversalHelper.copyLabels(currentStep, tinkerGraphStep, false); - traversal.removeStep(currentStep); currentStep = currentStep.getNextStep(); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/02cffb58/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java index 35f936c..c54a6b7 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java @@ -29,7 +29,9 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.FilterRankingStrategy; import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.InlineFilterStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.LazyBarrierStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies; +import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversal; import org.apache.tinkerpop.gremlin.structure.Vertex; import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.sideEffect.TinkerGraphStep; import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph; @@ -88,8 +90,17 @@ public class TinkerGraphStepStrategyTest { return traversal.addStep(graphStep); } + private static GraphStep<?, ?> V(final Object... hasKeyValues) { + final TinkerGraphStep<Vertex, Vertex> graphStep = new TinkerGraphStep<>(new GraphStep<>(EmptyTraversal.instance(), Vertex.class, true)); + for (int i = 0; i < hasKeyValues.length; i = i + 2) { + graphStep.addHasContainer(new HasContainer((String) hasKeyValues[i], (P) hasKeyValues[i + 1])); + } + return graphStep; + } + @Parameterized.Parameters(name = "{0}") public static Iterable<Object[]> generateTestParameters() { + final int LAZY_SIZE = 2500; return Arrays.asList(new Object[][]{ {__.V().out(), g_V().out(), Collections.emptyList()}, {__.V().has("name", "marko").out(), g_V("name", eq("marko")).out(), Collections.emptyList()}, @@ -112,7 +123,12 @@ public class TinkerGraphStepStrategyTest { {__.V().as("a").dedup().has("name", "marko").or(has("age", 10), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), g_V("name", eq("marko"), "age", eq(10).or(gt(32)), "name", eq("bob"), "lang", eq("java")).dedup().as("a"), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, {__.V().has("name", "marko").or(not(has("age")), has("age", gt(32))).has("name", "bob").has("lang", "java"), - g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(not(filter(properties("age"))), has("age", gt(32))), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()} + g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(not(filter(properties("age"))), has("age", gt(32))), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + /////// + {__.V().out().out().V().has("name", "marko").out(), g_V().out().barrier(LAZY_SIZE).out().barrier(LAZY_SIZE).asAdmin().addStep(V("name", eq("marko"))).barrier(LAZY_SIZE).out().barrier(LAZY_SIZE), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance(), LazyBarrierStrategy.instance())}, + {__.V().out().out().V().has("name", "marko").as("a").out(), g_V().out().barrier(LAZY_SIZE).out().barrier(LAZY_SIZE).asAdmin().addStep(V("name", eq("marko"))).barrier(LAZY_SIZE).as("a").out(), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance(), LazyBarrierStrategy.instance())}, + {__.V().out().V().has("age", gt(32)).barrier(10).has("name", "marko").as("a"), g_V().out().barrier(LAZY_SIZE).asAdmin().addStep(V("age", gt(32), "name", eq("marko"))).barrier(LAZY_SIZE).barrier(10).as("a"), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance(), LazyBarrierStrategy.instance())}, + {__.V().out().V().has("age", gt(32)).barrier(10).has("name", "marko").as("a"), g_V().out().barrier(LAZY_SIZE).asAdmin().addStep(V("age", gt(32), "name", eq("marko"))).barrier(LAZY_SIZE).barrier(10).as("a"), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, }); } }
