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/TINKERPOP-1602 Commit: da762dfee9b0ed05fd1185d80403e1be41873b58 Parents: e3889bf Author: Ted Wilmes <twil...@gmail.com> Authored: Tue Jan 24 14:57:07 2017 -0600 Committer: Ted Wilmes <twil...@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} }); } }
