tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From twil...@apache.org
Subject [1/5] tinkerpop git commit: Fixed a bug where keepLabels were not being pushed down into child traversals by PathRetractionStrategy.
Date Fri, 27 Jan 2017 20:58:42 GMT
Repository: tinkerpop
Updated Branches:
  refs/heads/tp32 c958b220b -> f0875d7be


Fixed a bug where keepLabels were not being pushed down into child traversals by PathRetractionStrategy.


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

Branch: refs/heads/tp32
Commit: da762dfee9b0ed05fd1185d80403e1be41873b58
Parents: e3889bf
Author: Ted Wilmes <twilmes@gmail.com>
Authored: Tue Jan 24 14:57:07 2017 -0600
Committer: Ted Wilmes <twilmes@gmail.com>
Committed: Tue Jan 24 14:57:07 2017 -0600

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../optimization/PathRetractionStrategy.java    | 58 +++++++++++++++++---
 .../PathRetractionStrategyTest.java             | 19 +++----
 3 files changed, 60 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/da762dfe/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 9453158..95cfb71 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.4 (Release Date: NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* Fixed a bug where `keepLabels` were not being pushed down into child traversals by `PathRetractionStrategy`.
 * Fixed a bug associated with user-provided maps and `GroupSideEffectStep`.
 * `GroupBiOperator` no longer maintains a detached traversal and thus, no more side-effect
related OLAP inconsistencies.
 * Added `ProjectedTraverser` which wraps a traverser with a `List<Object>` of projected
data.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/da762dfe/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
index fcc22a4..fc7eb8a 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
@@ -24,6 +24,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.step.Barrier;
 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.branch.RepeatStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DedupGlobalStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep;
@@ -33,6 +34,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversal
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
 import org.apache.tinkerpop.gremlin.process.traversal.util.PathUtil;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
+import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
 import org.javatuples.Pair;
 
 import java.util.ArrayList;
@@ -96,8 +98,12 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
                 if (currentStep instanceof MatchStep && (currentStep.getNextStep().equals(EmptyStep.instance())
|| currentStep.getNextStep() instanceof DedupGlobalStep)) {
                     pathProcessor.setKeepLabels(((MatchStep) currentStep).getMatchStartLabels());
                     pathProcessor.getKeepLabels().addAll(((MatchStep) currentStep).getMatchEndLabels());
-                } else
-                    pathProcessor.setKeepLabels(new HashSet<>(keepLabels));
+                } else {
+                    if (pathProcessor.getKeepLabels() == null)
+                        pathProcessor.setKeepLabels(new HashSet<>(keepLabels));
+                    else
+                        pathProcessor.getKeepLabels().addAll(new HashSet<>(keepLabels));
+                }
 
                 if (currentStep.getTraversal().getParent() instanceof MatchStep) {
                     pathProcessor.setKeepLabels(((MatchStep) currentStep.getTraversal().getParent().asStep()).getMatchStartLabels());
@@ -111,6 +117,7 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
                         !(currentStep.getNextStep() instanceof Barrier) &&
                         !(currentStep.getTraversal().getParent() instanceof MatchStep) &&
                         (!(currentStep.getNextStep() instanceof EmptyStep) || TraversalHelper.isGlobalChild(currentStep.getTraversal())))
+
                     TraversalHelper.insertAfterStep(new NoOpBarrierStep<>(traversal,
this.standardBarrierSize), currentStep, traversal);
             }
         }
@@ -141,16 +148,29 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
                 hasRepeat = true;
             }
 
+            // if parent step is a TraversalParent itself and it has more than 1 child traversal
+            // propagate labels to children
+            if (step instanceof TraversalParent) {
+                final List<Traversal.Admin<Object, Object>> children = new ArrayList<>();
+                children.addAll(((TraversalParent) step).getGlobalChildren());
+                children.addAll(((TraversalParent) step).getLocalChildren());
+                // if this is the only child traversal, do not re-push labels
+                if (children.size() > 1)
+                    applyToChildren(keepLabels, children);
+            }
+
             // propagate requirements of keep labels back through the traversal's previous
steps
             // to ensure that the label is not dropped before it reaches the step(s) that
require it
             step = step.getPreviousStep();
             while (!(step.equals(EmptyStep.instance()))) {
                 if (step instanceof PathProcessor) {
-                    if (((PathProcessor) step).getKeepLabels() == null) {
-                        ((PathProcessor) step).setKeepLabels(new HashSet<>(keepLabels));
-                    } else {
-                        ((PathProcessor) step).getKeepLabels().addAll(new HashSet<>(keepLabels));
-                    }
+                    addLabels((PathProcessor) step, keepLabels);
+                }
+                if (step instanceof TraversalParent) {
+                    final List<Traversal.Admin<Object, Object>> children = new
ArrayList<>();
+                    children.addAll(((TraversalParent) step).getGlobalChildren());
+                    children.addAll(((TraversalParent) step).getLocalChildren());
+                    applyToChildren(keepLabels, children);
                 }
                 step = step.getPreviousStep();
             }
@@ -190,8 +210,30 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
         }
     }
 
+    private void applyToChildren(Set<String> keepLabels, List<Traversal.Admin<Object,
Object>> children) {
+        for (Traversal.Admin<Object, Object> child : children) {
+            TraversalHelper.applyTraversalRecursively(trav -> addLabels(trav, keepLabels),
child);
+        }
+    }
+
+    private void addLabels(final Traversal.Admin traversal, final Set<String> keepLabels)
{
+        for (Object s : traversal.getSteps()) {
+            if (s instanceof PathProcessor) {
+                addLabels((PathProcessor) s, keepLabels);
+            }
+        }
+    }
+
+    private void addLabels(PathProcessor s, Set<String> keepLabels) {
+        if (s.getKeepLabels() == null) {
+            s.setKeepLabels(new HashSet<>(keepLabels));
+        } else {
+            s.getKeepLabels().addAll(new HashSet<>(keepLabels));
+        }
+    }
+
     @Override
     public Set<Class<? extends OptimizationStrategy>> applyPrior() {
         return PRIORS;
     }
-}
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/da762dfe/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
index fc1bc10..71b0ad5 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java
@@ -37,14 +37,8 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
 
-import static org.apache.tinkerpop.gremlin.process.traversal.P.eq;
-import static org.apache.tinkerpop.gremlin.process.traversal.P.gte;
-import static org.apache.tinkerpop.gremlin.process.traversal.P.neq;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.as;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.limit;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.select;
-import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.values;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.*;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.*;
 import static org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.PathRetractionStrategy.MAX_BARRIER_SIZE;
 import static org.junit.Assert.assertEquals;
 
@@ -153,7 +147,7 @@ public class PathRetractionStrategyTest {
                         as("c").out().as("d", "e").select("c", "e").out().select("c"),
                         as("c").out().as("d", "e").select("c", "e").out().select("c")).
                         out().select("c"),
-                        "[[b, c, e], [c, e], [[c], [c]], [[c], [c]], []]", null},
+                        "[[b, c, e], [c, e], [[c, e], [c, e]], [[c, e], [c, e]], []]", null},
                 {__.V().as("a").out().as("b").select("a").select("b").
                         local(as("c").out().as("d", "e").select("c", "e").out().select("c")).
                         out().select("c"),
@@ -180,7 +174,12 @@ public class PathRetractionStrategyTest {
                 {__.V().out("created").project("a", "b").by("name").by(__.in("created").count()).order().by(select("b")).select("a"),
"[[[a]], []]", null},
                 {__.order().by("weight", Order.decr).store("w").by("weight").filter(values("weight").as("cw").
                         select("w").by(limit(Scope.local, 1)).as("mw").where("cw", eq("mw"))).project("from",
"to", "weight").by(__.outV()).by(__.inV()).by("weight"),
-                        "[[[cw, mw], []]]", null}
+                        "[[[cw, mw], []]]", null},
+                {__.V().limit(1).as("z").out().repeat(store("seen").out().where(without("seen"))).until(where(eq("z"))),
+                        "[[[z, seen]], [[z, seen]]]", null},
+                {__.V().as("a").optional(bothE().dedup().as("b")).
+                        choose(select("b"), select("a","b"), project("a").by(select("a"))),
+                        "[[[a, b]], [[a, b]], [[a, b]], [[[a, b]]], [[a, b]]]", null}
         });
     }
 }


Mime
View raw message