You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2019/10/23 18:40:19 UTC
[tinkerpop] branch TINKERPOP-1568 updated: TINKERPOP-1568 Changed
when side-effects were set to child traversals in strategy application
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-1568
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/TINKERPOP-1568 by this push:
new 2e2c391 TINKERPOP-1568 Changed when side-effects were set to child traversals in strategy application
2e2c391 is described below
commit 2e2c391cdd04d59d3db6eea78258960e8cd8abe9
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Wed Oct 23 14:34:11 2019 -0400
TINKERPOP-1568 Changed when side-effects were set to child traversals in strategy application
We were doing this "before" strategy application but it caused trouble with strategies like TranslationStrategy which modified side-effects as part of their execution. That said, TranslationStrategy is a "test" strategy and I wonder if we'd actually see that kind of thing happening in a normal strategy implementation. Moving when side-effects were set not only fixed the problem but also made me question whether the parent strategy needed to be set to all the children. Removing that al [...]
---
.../process/traversal/util/DefaultTraversal.java | 11 +++++----
.../process/traversal/step/ComplexTest.java | 27 +++++++++++-----------
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
index 277edfc..592f46c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
@@ -131,11 +131,13 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
// application of the strategies we need to call applyStrategies() on all the children to ensure that their
// steps get reId'd and traverser requirements are set.
if (isRoot() || this.getParent() instanceof VertexProgramStep) {
- TraversalHelper.applyTraversalRecursively(t -> {
- t.setStrategies(this.strategies);
- t.setSideEffects(this.sideEffects);
- }, this);
+ // note that prior to applying strategies to children we used to set side-effects and strategies of all
+ // children to that of the parent. under this revised model of strategy application from TINKERPOP-1568
+ // it doesn't appear to be necessary to do that (at least from the perspective of the test suite). by,
+ // moving side-effect setting after actual recursive strategy application we save a loop and by
+ // consequence also fix a problem where strategies might reset something in sideeffects which seems to
+ // happen in TranslationStrategy.
final Iterator<TraversalStrategy<?>> strategyIterator = this.strategies.toIterator();
while (strategyIterator.hasNext()) {
final TraversalStrategy<?> strategy = strategyIterator.next();
@@ -147,6 +149,7 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
if (hasGraph) t.setGraph(this.graph);
if(!(t.isRoot()) && t != this && !t.isLocked()) {
t.setStrategies(new DefaultTraversalStrategies());
+ t.setSideEffects(this.sideEffects);
t.applyStrategies();
}
}, this);
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java
index 2de9d97..f7735c6 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ComplexTest.java
@@ -185,7 +185,6 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
@Test
@LoadGraphWith(MODERN)
- @org.junit.Ignore("temporary")
public void allShortestPaths() {
final Traversal<Vertex, List<Object>> traversal = getAllShortestPaths();
printTraversalForm(traversal);
@@ -318,27 +317,27 @@ public abstract class ComplexTest extends AbstractGremlinProcessTest {
return g.withoutStrategies(ComputerVerificationStrategy.class, PathRetractionStrategy.class).
V().as("v").both().as("v").
project("src", "tgt", "p").
- by(select(first, "v")).
- by(select(last, "v")).
- by(select(all, "v")).as("triple").
+ by(select(first, "v")).
+ by(select(last, "v")).
+ by(select(all, "v")).as("triple").
group("x").
- by(select("src", "tgt")).
- by(select("p").fold()).select("tgt").barrier().
+ by(select("src", "tgt")).
+ by(select("p").fold()).select("tgt").barrier().
repeat(both().as("v").
project("src", "tgt", "p").
- by(select(first, "v")).
- by(select(last, "v")).
- by(select(all, "v")).as("t").
+ by(select(first, "v")).
+ by(select(last, "v")).
+ by(select(all, "v")).as("t").
filter(select(all, "p").count(local).as("l").
- select(last, "t").select(all, "p").dedup(local).count(local).where(eq("l"))).
+ select(last, "t").select(all, "p").dedup(local).count(local).where(eq("l"))).
select(last, "t").
not(select(all, "p").as("p").count(local).as("l").
- select(all, "x").unfold().filter(select(keys).where(eq("t")).by(select("src", "tgt"))).
- filter(select(values).unfold().or(count(local).where(lt("l")), where(eq("p"))))).
+ select(all, "x").unfold().filter(select(keys).where(eq("t")).by(select("src", "tgt"))).
+ filter(select(values).unfold().or(count(local).where(lt("l")), where(eq("p"))))).
barrier().
group("x").
- by(select("src", "tgt")).
- by(select(all, "p").fold()).select("tgt").barrier()).
+ by(select("src", "tgt")).
+ by(select(all, "p").fold()).select("tgt").barrier()).
cap("x").select(values).unfold().unfold().map(unfold().id().fold());
}