You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by ok...@apache.org on 2015/05/13 21:21:17 UTC

incubator-tinkerpop git commit: IdentityRemovalStrategy is now smart about step labels. Updated traversal docs with a CAUTION about VendorOptimizations vs. Optimizations.

Repository: incubator-tinkerpop
Updated Branches:
  refs/heads/master c00ca0c79 -> 7fe1aef5b


IdentityRemovalStrategy is now smart about step labels. Updated traversal docs with a CAUTION about VendorOptimizations vs. Optimizations.


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

Branch: refs/heads/master
Commit: 7fe1aef5b13e19f73cec92d26320d422720bb01e
Parents: c00ca0c
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Wed May 13 13:21:13 2015 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Wed May 13 13:21:13 2015 -0600

----------------------------------------------------------------------
 docs/src/the-traversal.asciidoc                  | 17 ++++++++++++-----
 .../optimization/IdentityRemovalStrategy.java    | 19 ++++++++++++++-----
 .../IdentityRemovalStrategyTest.java             | 11 ++++++-----
 3 files changed, 32 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/7fe1aef5/docs/src/the-traversal.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/the-traversal.asciidoc b/docs/src/the-traversal.asciidoc
index 8acd4dd..56b1c7e 100644
--- a/docs/src/the-traversal.asciidoc
+++ b/docs/src/the-traversal.asciidoc
@@ -1503,7 +1503,7 @@ A simple `OptimizationStrategy` is the `IdentityRemovalStrategy`.
 
 [source,java]
 ----
-public class IdentityRemovalStrategy extends AbstractTraversalStrategy<TraversalStrategy.OptimizationStrategy> implements TraversalStrategy.OptimizationStrategy {
+public final class IdentityRemovalStrategy extends AbstractTraversalStrategy<TraversalStrategy.OptimizationStrategy> implements TraversalStrategy.OptimizationStrategy {
 
     private static final IdentityRemovalStrategy INSTANCE = new IdentityRemovalStrategy();
 
@@ -1514,9 +1514,14 @@ public class IdentityRemovalStrategy extends AbstractTraversalStrategy<Traversal
     public void apply(final Traversal.Admin<?, ?> traversal) {
         if (!TraversalHelper.hasStepOfClass(IdentityStep.class, traversal))
             return;
-        TraversalHelper.getStepsOfClass(IdentityStep.class, traversal).stream()
-                .filter(step -> !TraversalHelper.isLabeled(step))
-                .forEach(step -> traversal.removeStep(step));
+
+        TraversalHelper.getStepsOfClass(IdentityStep.class, traversal).stream().forEach(identityStep -> {
+            final Step<?, ?> previousStep = identityStep.getPreviousStep();
+            if (!(previousStep instanceof EmptyStep) || identityStep.getLabels().isEmpty()) {
+                ((IdentityStep<?>) identityStep).getLabels().forEach(previousStep::addLabel);
+                traversal.removeStep(identityStep);
+            }
+        });
     }
 
     public static IdentityRemovalStrategy instance() {
@@ -1525,7 +1530,7 @@ public class IdentityRemovalStrategy extends AbstractTraversalStrategy<Traversal
 }
 ----
 
-This strategy simply removes any unlabeled `IdentityStep` steps in the Traversal as `aStep().identity().identity().bStep()` is equivalent to `aStep().bStep()`. For those traversal strategies that require other strategies to execute prior or post to the strategy, then the following two methods can be defined in `TraversalStrategy` (with defaults being an empty set). If the `TraversalStrategy` is in a particular traversal category (i.e. decoration, optimization, finalization, or verification), then priors and posts are only possible within the category.
+This strategy simply removes any `IdentityStep` steps in the Traversal as `aStep().identity().identity().bStep()` is equivalent to `aStep().bStep()`. For those traversal strategies that require other strategies to execute prior or post to the strategy, then the following two methods can be defined in `TraversalStrategy` (with defaults being an empty set). If the `TraversalStrategy` is in a particular traversal category (i.e. decoration, optimization, vendor-optimization, finalization, or verification), then priors and posts are only possible within the category.
 
 [source,java]
 public Set<Class<? extends S>> applyPrior();
@@ -1590,6 +1595,8 @@ t.iterate(); null
 t.toString()
 ----
 
+CAUTION: The reason that `OptimizationStrategy` and `VendorOptimizationStrategy` are two different categories is that optimization strategies should only rewrite the traversal using TinkerPop3 steps. This ensures that the optimizations executed at the end of the optimization strategy round are TinkerPop3 compliant. From there, vendor optimizations can analyze the traversal and rewrite the traversal as desired using vendor specific steps (e.g. replacing `GraphStep.HasStep...HasStep` with `TinkerGraphStep`). If vendor's optimizations use vendor-specific steps and implement `OptimizationStrategy`, then other TinkerPop3 optimizations may fail to optimize the traversal or mis-understand the vendor-specific step behaviors (e.g. `VendorVertexStep extends VertexStep`) and yield incorrect semantics.
+
 A collection of useful `DecorationStrategy` strategies are provided with TinkerPop3 and are generally useful to end-users.  The following sub-sections detail these strategies:
 
 ElementIdStrategy

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/7fe1aef5/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategy.java
index 7751a5b..47f60e3 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategy.java
@@ -18,20 +18,25 @@
  */
 package org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization;
 
+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.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;
 
 /**
- * IdentityRemovalStrategy looks for unlabeled {@link IdentityStep} instances and removes them.
+ * IdentityRemovalStrategy looks for {@link IdentityStep} instances and removes them.
+ * If the identity step is labeled, its labels are added to the previous step.
+ * If the identity step is labeled and its the first step, in the traversal, it stays.
  * <p/>
  *
  * @author Marko A. Rodriguez (http://markorodriguez.com)
  * @example <pre>
  * __.out().identity().count()            // is replaced by __.out().count()
- * __.identity().as("a").in().identity()  // is replaced by __.identity().as("a").in()
+ * __.in().identity().as("a")             // is replaced by __.in().as("a")
+ * __.identity().as("a").out()            // is replaced by __.identity().as("a").out()
  * </pre>
  */
 public final class IdentityRemovalStrategy extends AbstractTraversalStrategy<TraversalStrategy.OptimizationStrategy> implements TraversalStrategy.OptimizationStrategy {
@@ -46,9 +51,13 @@ public final class IdentityRemovalStrategy extends AbstractTraversalStrategy<Tra
         if (!TraversalHelper.hasStepOfClass(IdentityStep.class, traversal))
             return;
 
-        TraversalHelper.getStepsOfClass(IdentityStep.class, traversal).stream()
-                .filter(step -> step.getLabels().isEmpty())
-                .forEach(traversal::removeStep);
+        TraversalHelper.getStepsOfClass(IdentityStep.class, traversal).stream().forEach(identityStep -> {
+            final Step<?, ?> previousStep = identityStep.getPreviousStep();
+            if (!(previousStep instanceof EmptyStep) || identityStep.getLabels().isEmpty()) {
+                ((IdentityStep<?>) identityStep).getLabels().forEach(previousStep::addLabel);
+                traversal.removeStep(identityStep);
+            }
+        });
     }
 
     public static IdentityRemovalStrategy instance() {

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/7fe1aef5/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategyTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategyTest.java
index e0950ae..a7f6547 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/IdentityRemovalStrategyTest.java
@@ -118,11 +118,12 @@ public class IdentityRemovalStrategyTest {
         static Iterable<Object[]> generateTestParameters() {
 
             return Arrays.asList(new Traversal[][]{
-                    {__.identity().out(),__.out()},
-                    {__.identity().out().identity(),__.out()},
-                    {__.identity().as("a").out().identity(),__.identity().as("a").out()},
-                    {__.identity().as("a").out().identity().as("b"),__.identity().as("a").out().identity().as("b")},
-                    {__.identity().as("a").out().in().identity().identity().as("b").identity().out(),__.identity().as("a").out().in().identity().as("b").out()},
+                    {__.identity().out(), __.out()},
+                    {__.identity().out().identity(), __.out()},
+                    {__.identity().as("a").out().identity(), __.identity().as("a").out()},
+                    {__.identity().as("a").out().identity().as("b"), __.identity().as("a").out().as("b")},
+                    {__.identity().as("a").out().in().identity().identity().as("b").identity().out(), __.identity().as("a").out().in().as("b").out()},
+                    {__.out().identity().as("a").out().in().identity().identity().as("b").identity().out(), __.out().as("a").out().in().as("b").out()},
             });
         }
     }