From c167d9ea779ee2229e5cd56e35a37a6fd225ffc9 Mon Sep 17 00:00:00 2001 From: "Marko A. Rodriguez" Date: Mon, 28 Aug 2017 10:58:34 -0600 Subject: [PATCH 1/2] tweaked how TraversalParent children are processed in ComputerVerifiationStrategy. Given the outside-in nature of Gremlin compilation, we were analyzing child traversals that were, in fact, not compiled yet. We now check for local star graph issues on a per traversal level. I can't believe we didn't run into other problems before this. --- .../ComputerVerificationStrategy.java | 24 ++++++------------- .../ComputerVerificationStrategyTest.java | 4 ++++ 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java index 3a3c6e2175d..2d076b6541b 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategy.java @@ -26,7 +26,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; 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.map.GraphStep; import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.InjectStep; import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.SubgraphStep; @@ -38,7 +37,6 @@ import java.util.Arrays; import java.util.HashSet; -import java.util.Optional; import java.util.Set; /** @@ -56,7 +54,6 @@ private ComputerVerificationStrategy() { @Override public void apply(final Traversal.Admin traversal) { - if (!TraversalHelper.onGraphComputer(traversal)) return; @@ -67,27 +64,20 @@ public void apply(final Traversal.Admin traversal) { throw new VerificationException("Profiling a multi-VertexProgramStep traversal is currently not supported on GraphComputer", traversal); } - for (final Step step : traversal.getSteps()) { - - // you can not traverse past the local star graph with localChildren (e.g. by()-modulators). - if (step instanceof TraversalParent) { - final Optional> traversalOptional = ((TraversalParent) step).getLocalChildren().stream() - .filter(t -> !TraversalHelper.isLocalStarGraph(t.asAdmin())) - .findAny(); - if (traversalOptional.isPresent()) - throw new VerificationException("Local traversals may not traverse past the local star-graph on GraphComputer: " + traversalOptional.get(), traversal); - } + // this is a problem because sideEffect.merge() is transient on the OLAP reduction + if (TraversalHelper.getRootTraversal(traversal).getTraverserRequirements().contains(TraverserRequirement.ONE_BULK)) + throw new VerificationException("One bulk is currently not supported on GraphComputer: " + traversal, traversal); - // this is a problem because sideEffect.merge() is transient on the OLAP reduction - if (TraversalHelper.getRootTraversal(traversal).getTraverserRequirements().contains(TraverserRequirement.ONE_BULK)) - throw new VerificationException("One bulk is currently not supported on GraphComputer: " + step, traversal); + // you can not traverse past the local star graph with localChildren (e.g. by()-modulators). + if (!TraversalHelper.isGlobalChild(traversal) && !TraversalHelper.isLocalStarGraph(traversal)) + throw new VerificationException("Local traversals may not traverse past the local star-graph on GraphComputer: " + traversal, traversal); + for (final Step step : traversal.getSteps()) { if (step instanceof PathProcessor && ((PathProcessor) step).getMaxRequirement() != PathProcessor.ElementRequirement.ID) throw new VerificationException("It is not possible to access more than a path element's id on GraphComputer: " + step + " requires " + ((PathProcessor) step).getMaxRequirement(), traversal); if (UNSUPPORTED_STEPS.stream().filter(c -> c.isAssignableFrom(step.getClass())).findFirst().isPresent()) throw new VerificationException("The following step is currently not supported on GraphComputer: " + step, traversal); - } Step nextParentStep = traversal.getParent().asStep(); diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategyTest.java index da41881c7c5..e594bd4e4de 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ComputerVerificationStrategyTest.java @@ -23,6 +23,8 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.ConnectiveStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.AdjacentToIncidentStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies; import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversal; import org.junit.Test; @@ -34,6 +36,7 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.max; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.min; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out; +import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.outE; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.sum; import static org.junit.Assert.fail; @@ -53,6 +56,7 @@ public static Iterable data() { {"__.values(\"age\").union(max(), min(), sum())", __.values("age").union(max(), min(), sum()), true}, {"__.count().sum()", __.count().sum(), true}, {"__.where(\"a\",eq(\"b\")).out()", __.where("a", P.eq("b")).out(), true}, + {"__.where(and(outE(\"knows\"),outE(\"created\"))).values(\"name\")", __.where(__.and(outE("knows"), outE("created"))).values("name"), true}, }); } From eb1be4dcfb6053ff65aae6805db61e6556c22384 Mon Sep 17 00:00:00 2001 From: "Marko A. Rodriguez" Date: Mon, 28 Aug 2017 11:04:12 -0600 Subject: [PATCH 2/2] updated CHANGELOG. --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 9fff5350f66..1ffa315cba7 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.2.7 (Release Date: NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Fixed a bug in `ComputerVerificationStrategy` where child traversals were being analyzed prior to compilation. * Fixed a bug that prevented Gremlin from ordering lists and streams made of mixed number types.