Repository: tinkerpop Updated Branches: refs/heads/TINKERPOP-1473 a69efffef -> 301975739
added a new WhereTest and realized a labeling issue. Labeling issue fixed. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/30197573 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/30197573 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/30197573 Branch: refs/heads/TINKERPOP-1473 Commit: 301975739d47c2b331c2a9292a34ca817255f587 Parents: a69efff Author: Marko A. Rodriguez <okramma...@gmail.com> Authored: Wed Nov 2 05:37:41 2016 -0600 Committer: Marko A. Rodriguez <okramma...@gmail.com> Committed: Wed Nov 2 05:37:41 2016 -0600 ---------------------------------------------------------------------- .../optimization/PathProcessorStrategy.java | 35 +++++++++++++++++--- .../optimization/PathProcessorStrategyTest.java | 22 +++++++----- .../step/filter/GroovyWhereTest.groovy | 5 +++ .../traversal/step/filter/WhereTest.java | 15 +++++++++ 4 files changed, 64 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30197573/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategy.java index c9d49a9..fa0ddc5 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategy.java @@ -23,20 +23,28 @@ import org.apache.tinkerpop.gremlin.process.traversal.Pop; import org.apache.tinkerpop.gremlin.process.traversal.Step; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.step.LambdaHolder; import org.apache.tinkerpop.gremlin.process.traversal.step.PathProcessor; import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WhereTraversalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.PathStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.SelectOneStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.SelectStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.TraversalMapStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.TreeStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.IdentityStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep; import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; /** * PathProcessStrategy is an OLAP strategy that does its best to turn non-local children in {@code where()} and {@code select()} @@ -47,14 +55,18 @@ import java.util.Set; * @example <pre> * __.select(a).by(x) // is replaced by select(a).map(x) * __.select(a,b).by(x).by(y) // is replaced by select(a).by(x).as(a).select(b).by(y).as(b).select(a,b) - * __.where(as(a).out().as(b)) // is replaced by select(a).where(out().as(b)) - * __.where(as(a).out()) // is replaced by select(a).filter(out()) + * __.where(as(a).out().as(b)) // is replaced by as(xyz).select(a).where(out().as(b)).select(xyz) + * __.where(as(a).out()) // is replaced by as(xyz).select(a).filter(out()).select(xyz) * </pre> */ public final class PathProcessorStrategy extends AbstractTraversalStrategy<TraversalStrategy.OptimizationStrategy> implements TraversalStrategy.OptimizationStrategy { private static final PathProcessorStrategy INSTANCE = new PathProcessorStrategy(); + private static final boolean IS_TESTING = Boolean.valueOf(System.getProperty("is.testing", "false")); + + private static final Set<Class> INVALIDATING_STEP_CLASSES = new HashSet<>(Arrays.asList(PathStep.class, TreeStep.class, LambdaHolder.class)); + private PathProcessorStrategy() { } @@ -70,7 +82,9 @@ public final class PathProcessorStrategy extends AbstractTraversalStrategy<Trave @Override public void apply(final Traversal.Admin<?, ?> traversal) { - if (!TraversalHelper.onGraphComputer(traversal) || !TraversalHelper.isGlobalChild(traversal)) + if (!TraversalHelper.onGraphComputer(traversal) || + !TraversalHelper.isGlobalChild(traversal) || + TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES, TraversalHelper.getRootTraversal(traversal))) // TODO: use the MARKER model when that PR is merged return; // process where(as("a").out()...) => select("a").where(out()...) @@ -90,11 +104,18 @@ public final class PathProcessorStrategy extends AbstractTraversalStrategy<Trave } } final WhereTraversalStep.WhereStartStep<?> whereStartStep = (WhereTraversalStep.WhereStartStep<?>) localChild.getStartStep(); - final int index = TraversalHelper.stepIndex(whereTraversalStep, traversal); + int index = TraversalHelper.stepIndex(whereTraversalStep, traversal); final SelectOneStep<?, ?> selectOneStep = new SelectOneStep<>(traversal, Pop.last, whereStartStep.getScopeKeys().iterator().next()); traversal.addStep(index, selectOneStep); + final String generatedLabel = PathProcessorStrategy.generateLabel(); + if (selectOneStep.getPreviousStep() instanceof EmptyStep) { + TraversalHelper.insertBeforeStep(new IdentityStep<>(traversal), selectOneStep, traversal); + index++; + } + selectOneStep.getPreviousStep().addLabel(generatedLabel); + TraversalHelper.insertAfterStep(new SelectOneStep<>(traversal, Pop.last, generatedLabel), whereTraversalStep, traversal); whereStartStep.removeScopeKey(); - // process where(as("a").out()) => select("a").filter(out()) + // process where(as("a").out()) => as('xyz').select("a").filter(out()).select('xyz') if (!(localChild.getEndStep() instanceof WhereTraversalStep.WhereEndStep)) { localChild.removeStep(localChild.getStartStep()); traversal.addStep(index + 1, new TraversalFilterStep<>(traversal, localChild)); @@ -154,6 +175,10 @@ public final class PathProcessorStrategy extends AbstractTraversalStrategy<Trave return INSTANCE; } + private static String generateLabel() { + return IS_TESTING ? "xyz" : UUID.randomUUID().toString(); + } + private static int labelCount(final String label, final Traversal.Admin<?, ?> traversal) { int count = 0; for (final Step step : traversal.getSteps()) { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30197573/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java index 562d5d6..89edd85 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java @@ -25,11 +25,12 @@ import org.apache.tinkerpop.gremlin.process.traversal.Pop; 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.__; import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal; import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies; -import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversal; import org.apache.tinkerpop.gremlin.structure.Graph; import org.junit.Test; import org.junit.runner.RunWith; @@ -58,7 +59,10 @@ public class PathProcessorStrategyTest { @Test public void doTest() { - this.original.asAdmin().setParent(new TraversalVertexProgramStep(EmptyTraversal.instance(), EmptyTraversal.instance())); // trick it + final Traversal.Admin<?, ?> rootTraversal = new DefaultGraphTraversal<>(); + final TraversalParent parent = new TraversalVertexProgramStep(rootTraversal, this.original.asAdmin()); + rootTraversal.addStep(parent.asStep()); + this.original.asAdmin().setParent(parent); // trick it into OLAP final TraversalStrategies strategies = new DefaultTraversalStrategies(); strategies.addStrategies(PathProcessorStrategy.instance()); for (final TraversalStrategy strategy : this.otherStrategies) { @@ -81,7 +85,7 @@ public class PathProcessorStrategyTest { {__.select("a").by("name"), __.select("a").map(new ElementValueTraversal<>("name")), Collections.emptyList()}, {__.select("a").out(), __.select("a").out(), Collections.emptyList()}, {__.select(Pop.all, "a").by(__.values("name")), __.select(Pop.all, "a").by(__.values("name")), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, - {__.select(Pop.last, "a").by(__.values("name")), __.select(Pop.last, "a").map(__.values("name")),TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, + {__.select(Pop.last, "a").by(__.values("name")), __.select(Pop.last, "a").map(__.values("name")), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, {__.select(Pop.first, "a").by(__.values("name")), __.select(Pop.first, "a").map(__.values("name")), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, // select("a","b") {__.select("a", "b"), __.select("a", "b"), Collections.emptyList()}, @@ -95,11 +99,13 @@ public class PathProcessorStrategyTest { {__.select(Pop.all, "a", "b").by("name").by("age"), __.select(Pop.all, "a", "b").by("name").by("age"), Collections.emptyList()}, // where(as("a")...) {__.where(__.out("knows")), __.where(__.outE("knows")), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, - {__.where(__.as("a").out("knows")), __.select(Pop.last, "a").filter(__.out("knows")), Collections.emptyList()}, - {__.where(__.as("a").has("age",P.gt(10))), __.select(Pop.last, "a").has("age",P.gt(10)), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, - {__.select("b").where(__.as("a").has("age",P.gt(10))), __.select("b").select(Pop.last, "a").has("age",P.gt(10)), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, - {__.where(__.as("a").out("knows").as("b")), __.select(Pop.last, "a").where(__.out("knows").as("b")), Collections.emptyList()}, - {__.where("a", P.eq("b")), __.where("a", P.eq("b")), Collections.emptyList()} + {__.where(__.as("a").out("knows")), __.identity().as("xyz").select(Pop.last, "a").filter(__.out("knows")).select(Pop.last, "xyz"), Collections.emptyList()}, + {__.where(__.as("a").has("age", P.gt(10))), __.identity().as("xyz").select(Pop.last, "a").has("age", P.gt(10)).select(Pop.last, "xyz"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, + {__.select("b").where(__.as("a").has("age", P.gt(10))), __.select("b").as("xyz").select(Pop.last, "a").has("age", P.gt(10)).select(Pop.last, "xyz"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, + {__.where(__.as("a").out("knows").as("b")), __.identity().as("xyz").select(Pop.last, "a").where(__.out("knows").as("b")).select(Pop.last, "xyz"), Collections.emptyList()}, + {__.where("a", P.eq("b")), __.where("a", P.eq("b")), Collections.emptyList()}, + {__.as("a").out().where(__.as("a").has("age", P.gt(10))).in(), __.as("a").out().as("xyz").select(Pop.last, "a").has("age", P.gt(10)).select(Pop.last, "xyz").in(), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, + {__.as("a").out().where(__.as("a").has("age", P.gt(10))).in().path(), __.as("a").out().where(__.as("a").has("age", P.gt(10))).in().path(), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, }); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30197573/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyWhereTest.groovy ---------------------------------------------------------------------- diff --git a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyWhereTest.groovy b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyWhereTest.groovy index a28c3dc..9348524 100644 --- a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyWhereTest.groovy +++ b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyWhereTest.groovy @@ -52,6 +52,11 @@ public abstract class GroovyWhereTest { new ScriptTraversal<>(g, "gremlin-groovy", "g.V().has('age').as('a').out.in.has('age').as('b').select('a','b').where(__.as('a').out('knows').as('b'))") } + @Override + public Traversal<Vertex, String> get_g_V_asXaX_outXcreatedX_whereXasXaX_name_isXjoshXX_inXcreatedX_name() { + new ScriptTraversal<>(g, "gremlin-groovy", "g.V.as('a').out('created').where(__.as('a').name.is('josh')).in('created').name") + } + /// where(global) @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/30197573/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTest.java ---------------------------------------------------------------------- diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTest.java index 03f76b7..ef2a5e4 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTest.java @@ -71,6 +71,8 @@ public abstract class WhereTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, Map<String, Object>> get_g_V_hasXageX_asXaX_out_in_hasXageX_asXbX_selectXa_bX_whereXa_outXknowsX_bX(); + public abstract Traversal<Vertex, String> get_g_V_asXaX_outXcreatedX_whereXasXaX_name_isXjoshXX_inXcreatedX_name(); + /// where(global) public abstract Traversal<Vertex, String> get_g_VX1X_asXaX_outXcreatedX_inXcreatedX_asXbX_whereXa_neqXbXX_name(final Object v1Id); @@ -202,6 +204,14 @@ public abstract class WhereTest extends AbstractGremlinProcessTest { assertFalse(traversal.hasNext()); } + @Test + @LoadGraphWith(MODERN) + public void g_V_asXaX_outXcreatedX_whereXasXaX_name_isXjoshXX_inXcreatedX_name() { + final Traversal<Vertex, String> traversal = get_g_V_asXaX_outXcreatedX_whereXasXaX_name_isXjoshXX_inXcreatedX_name(); + printTraversalForm(traversal); + checkResults(Arrays.asList("marko", "josh", "peter", "josh"), traversal); + } + /// where(global) @Test @@ -400,6 +410,11 @@ public abstract class WhereTest extends AbstractGremlinProcessTest { return g.V().has("age").as("a").out().in().has("age").as("b").select("a", "b").where(as("a").out("knows").as("b")); } + @Override + public Traversal<Vertex, String> get_g_V_asXaX_outXcreatedX_whereXasXaX_name_isXjoshXX_inXcreatedX_name() { + return g.V().as("a").out("created").where(as("a").values("name").is("josh")).in("created").values("name"); + } + /// where(global) @Override
