tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dkupp...@apache.org
Subject [1/3] tinkerpop git commit: Fixed bug in `IncidentToAdjacentStrategy`, it was missing some invalidating steps.
Date Thu, 12 Jan 2017 14:24:35 GMT
Repository: tinkerpop
Updated Branches:
  refs/heads/master e41d9ca00 -> b0be1d48e


Fixed bug in `IncidentToAdjacentStrategy`, it was missing some invalidating steps.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/35f936b0
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/35f936b0
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/35f936b0

Branch: refs/heads/master
Commit: 35f936b0e84d31c0d92600dbe129f54babb1900a
Parents: be5b64a
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Authored: Mon Jan 9 15:31:54 2017 +0100
Committer: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Committed: Mon Jan 9 15:47:27 2017 +0100

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../IncidentToAdjacentStrategy.java             | 61 +++++++++++---------
 .../IncidentToAdjacentStrategyTest.java         | 23 ++++----
 3 files changed, 47 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/35f936b0/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 89dcce4..54eef9d 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -569,6 +569,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 TinkerPop 3.1.6 (Release Date: NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* Fixed bug in `IncidentToAdjacentStrategy`, it was missing some invalidating steps.
 * Returned a confirmation on session close from Gremlin Server.
 * Use non-default port for running tests on Gremlin Server.
 * Fully shutdown metrics services in Gremlin Server on shutdown.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/35f936b0/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategy.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategy.java
index efe8618..4270ac3 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategy.java
@@ -23,11 +23,14 @@ 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.filter.CyclicPathStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.SimplePathStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.EdgeOtherVertexStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.EdgeVertexStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.PathStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.TreeStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.VertexStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.TreeSideEffectStep;
 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;
@@ -68,38 +71,13 @@ public final class IncidentToAdjacentStrategy extends AbstractTraversalStrategy<
         implements TraversalStrategy.OptimizationStrategy {
 
     private static final IncidentToAdjacentStrategy INSTANCE = new IncidentToAdjacentStrategy();
-    private static final Set<Class> INVALIDATING_STEP_CLASSES = new HashSet<>(Arrays.asList(PathStep.class,
TreeStep.class, LambdaHolder.class));
     private static final String MARKER = Graph.Hidden.hide("gremlin.incidentToAdjacent");
+    private static final Set<Class> INVALIDATING_STEP_CLASSES = new HashSet<>(Arrays.asList(CyclicPathStep.class,
+            PathStep.class, SimplePathStep.class, TreeStep.class, TreeSideEffectStep.class,
LambdaHolder.class));
 
     private IncidentToAdjacentStrategy() {
     }
 
-    @Override
-    public void apply(final Traversal.Admin<?, ?> traversal) {
-        // using a hidden label marker to denote whether the traversal should not be processed
by this strategy
-        if ((traversal.getParent() instanceof EmptyStep || traversal.getParent() instanceof
VertexProgramStep) &&
-                TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
traversal))
-            TraversalHelper.applyTraversalRecursively(t -> t.getStartStep().addLabel(MARKER),
traversal);
-        if (traversal.getStartStep().getLabels().contains(MARKER)) {
-            traversal.getStartStep().removeLabel(MARKER);
-            return;
-        }
-        ////////////////////////////////////////////////////////////////////////////
-        final Collection<Pair<VertexStep, Step>> stepsToReplace = new ArrayList<>();
-        Step prev = null;
-        for (final Step curr : traversal.getSteps()) {
-            if (isOptimizable(prev, curr)) {
-                stepsToReplace.add(Pair.with((VertexStep) prev, curr));
-            }
-            prev = curr;
-        }
-        if (!stepsToReplace.isEmpty()) {
-            for (final Pair<VertexStep, Step> pair : stepsToReplace) {
-                optimizeSteps(traversal, pair.getValue0(), pair.getValue1());
-            }
-        }
-    }
-
     /**
      * Checks whether a given step is optimizable or not.
      *
@@ -114,7 +92,8 @@ public final class IncidentToAdjacentStrategy extends AbstractTraversalStrategy<
             if (step1Dir.equals(Direction.BOTH)) {
                 return step2 instanceof EdgeOtherVertexStep;
             }
-            return step2 instanceof EdgeVertexStep && ((EdgeVertexStep) step2).getDirection().equals(step1Dir.opposite());
+            return step2 instanceof EdgeOtherVertexStep || (step2 instanceof EdgeVertexStep
&&
+                    ((EdgeVertexStep) step2).getDirection().equals(step1Dir.opposite()));
         }
         return false;
     }
@@ -141,6 +120,32 @@ public final class IncidentToAdjacentStrategy extends AbstractTraversalStrategy<
     }
 
     @Override
+    public void apply(final Traversal.Admin<?, ?> traversal) {
+        // using a hidden label marker to denote whether the traversal should not be processed
by this strategy
+        if ((traversal.getParent() instanceof EmptyStep || traversal.getParent() instanceof
VertexProgramStep) &&
+                TraversalHelper.hasStepOfAssignableClassRecursively(INVALIDATING_STEP_CLASSES,
traversal))
+            TraversalHelper.applyTraversalRecursively(t -> t.getStartStep().addLabel(MARKER),
traversal);
+        if (traversal.getStartStep().getLabels().contains(MARKER)) {
+            traversal.getStartStep().removeLabel(MARKER);
+            return;
+        }
+        ////////////////////////////////////////////////////////////////////////////
+        final Collection<Pair<VertexStep, Step>> stepsToReplace = new ArrayList<>();
+        Step prev = null;
+        for (final Step curr : traversal.getSteps()) {
+            if (isOptimizable(prev, curr)) {
+                stepsToReplace.add(Pair.with((VertexStep) prev, curr));
+            }
+            prev = curr;
+        }
+        if (!stepsToReplace.isEmpty()) {
+            for (final Pair<VertexStep, Step> pair : stepsToReplace) {
+                optimizeSteps(traversal, pair.getValue0(), pair.getValue1());
+            }
+        }
+    }
+
+    @Override
     public Set<Class<? extends OptimizationStrategy>> applyPrior() {
         return Collections.singleton(IdentityRemovalStrategy.class);
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/35f936b0/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategyTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategyTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategyTest.java
index 6e56ab8..abf1884 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IncidentToAdjacentStrategyTest.java
@@ -45,15 +45,6 @@ public class IncidentToAdjacentStrategyTest {
     @Parameterized.Parameter(value = 1)
     public Traversal optimized;
 
-    @Test
-    public void doTest() {
-        final TraversalStrategies strategies = new DefaultTraversalStrategies();
-        strategies.addStrategies(IncidentToAdjacentStrategy.instance());
-        this.original.asAdmin().setStrategies(strategies);
-        this.original.asAdmin().applyStrategies();
-        assertEquals(this.optimized, this.original);
-    }
-
     @Parameterized.Parameters(name = "{0}")
     public static Iterable<Object[]> generateTestParameters() {
 
@@ -67,12 +58,24 @@ public class IncidentToAdjacentStrategyTest {
                 {__.bothE().bothV(), __.bothE().bothV()},
                 {__.bothE().inV(), __.bothE().inV()},
                 {__.bothE().outV(), __.bothE().outV()},
+                {__.outE().otherV(), __.out()},
+                {__.inE().otherV(), __.in()},
                 {__.outE().as("a").inV(), __.outE().as("a").inV()}, // todo: this can be
optimized, but requires a lot more checks
                 {__.outE().inV().path(), __.outE().inV().path()},
-                {__.outE().inV().tree().as("a"), __.outE().inV().tree().as("a")},
+                {__.outE().inV().simplePath(), __.outE().inV().simplePath()},
+                {__.outE().inV().tree(), __.outE().inV().tree()},
                 {__.outE().inV().map(lambda), __.outE().inV().map(lambda)},
                 {__.union(__.outE().inV(), __.inE().outV()).path(), __.union(__.outE().inV(),
__.inE().outV()).path()},
                 {__.as("a").outE().inV().as("b"), __.as("a").out().as("b")}});
     }
+
+    @Test
+    public void doTest() {
+        final TraversalStrategies strategies = new DefaultTraversalStrategies();
+        strategies.addStrategies(IncidentToAdjacentStrategy.instance());
+        this.original.asAdmin().setStrategies(strategies);
+        this.original.asAdmin().applyStrategies();
+        assertEquals(this.optimized, this.original);
+    }
 }
 


Mime
View raw message