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/06/26 17:21:13 UTC

incubator-tinkerpop git commit: Removed MatchStep.getStartLabel(). The only class that needed it was MatchPredicateStrategy and that just uses the MatchStep.Helper.calculateStartLabel() static method. Added more MatchPredicateStrategyTest tests to make s

Repository: incubator-tinkerpop
Updated Branches:
  refs/heads/master 62172b034 -> 6e9376598


Removed MatchStep.getStartLabel(). The only class that needed it was MatchPredicateStrategy and that just uses the MatchStep.Helper.calculateStartLabel() static method. Added more MatchPredicateStrategyTest tests to make sure the compilation is correct. Tweaks to the-traversal.asciidoc formatting.


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

Branch: refs/heads/master
Commit: 6e93765989354ef469e65b1b9325422837a6b579
Parents: 62172b0
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Fri Jun 26 09:21:07 2015 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Fri Jun 26 09:21:07 2015 -0600

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  2 ++
 docs/src/the-traversal.asciidoc                 | 26 ++++++++++----------
 .../process/traversal/step/map/MatchStep.java   | 16 +++---------
 .../optimization/MatchPredicateStrategy.java    | 24 +++++++++++++++---
 .../process/traversal/util/TraversalHelper.java |  5 +---
 .../traversal/step/map/MatchStepTest.java       |  3 ---
 .../MatchPredicateStrategyTest.java             |  8 ++++--
 7 files changed, 47 insertions(+), 37 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/6e937659/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 9d86f1c..c96ac2a 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -25,6 +25,8 @@ image::http://www.tinkerpop.com/docs/current/images/gremlin-hindu.png[width=225]
 TinkerPop 3.0.0.GA (NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* `MatchStep` and `match()` no longer have a "start label" parameter -- it is computed if the incoming traverser does not have requisite labels.
+* Added `TraversalParent.removeGlobalChild()` and `TraversalParent.removeLocalChild()`.
 * Add a `clear` option to the Gephi Plugin to empty the Gephi workspace.
 * `AbstractStep` now guarantees that bulk-less and null-valued traversers are never propagated.
 * Added `dedup(string...)` which allows for the deduplication of a stream based on unique scope values.

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/6e937659/docs/src/the-traversal.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/the-traversal.asciidoc b/docs/src/the-traversal.asciidoc
index f5efcd9..019199b 100644
--- a/docs/src/the-traversal.asciidoc
+++ b/docs/src/the-traversal.asciidoc
@@ -800,15 +800,15 @@ g.V().match(
         __.as('b').match(
                      __.as('b').out('created').as('c'),
                      __.as('c').has('name','ripple')).
-                  select('c').as('c')).
+                   select('c').as('c')).
       select('a','c').by('name')
 ----
 
-If an extended, step-labeled traversal proceeds the `match()`-step and the traverser entering the `match()` is destined to bind to a particular variable, then the previous step should be labeled accordingly.
+If a step-labeled traversal proceeds the `match()`-step and the traverser entering the `match()` is destined to bind to a particular variable, then the previous step should be labeled accordingly.
 
 [gremlin-groovy,modern]
 ----
-g.V().as('a').out('knows').as('b')
+g.V().as('a').out('knows').as('b').
   match(
     __.as('b').out('created').as('c'),
     __.not(__.as('c').in('created').as('a'))).
@@ -827,13 +827,12 @@ If a variable is at the start of a traversal pattern it *must* exist as a label
 ----
 g.V().as('a').out().as('b'). <1>
     match( <2>
-        __.as('a').out().count().as('c'), <3>
-        __.not(__.as('a').in().as('b')), <4>
-        or( <5>
-          __.as('a').out('knows').as('b'),
-          __.as('b').in().count().as('c').and().as('c').is(gt(2))  <6>
-        )
-    ).dedup('a','c'). <7>
+      __.as('a').out().count().as('c'), <3>
+      __.not(__.as('a').in().as('b')), <4>
+      or( <5>
+        __.as('a').out('knows').as('b'),
+        __.as('b').in().count().as('c').and().as('c').is(gt(2)))).  <6>
+    dedup('a','c'). <7>
     select('a','b','c').by('name').by('name').by() <8>
 ----
 
@@ -857,7 +856,8 @@ Match is typically used in conjunction with both `select()` (demonstrated previo
 g.V().match(
         __.as('a').out('created').as('b'),
         __.as('b').in('created').as('c')).
-          where('a', neq('c')).select('a','c').by('name')
+        where('a', neq('c')).
+      select('a','c').by('name')
 ----
 
 The `where()`-step can take either a `P`-predicate (example above) or a `Traversal` (example below). Using `MatchPredicateStrategy`, `where()`-clauses are automatically folded into `match()` and thus, subject to the query optimizer within `match()`-step.
@@ -868,8 +868,8 @@ traversal = g.V().match(
                     __.as('a').has(label,'person'), <1>
                     __.as('a').out('created').as('b'),
                     __.as('b').in('created').as('c')).
-                      where(__.as('a').out('knows').as('c')). <2>
-                      select('a','c').by('name'); null <3>
+                    where(__.as('a').out('knows').as('c')). <2>
+                  select('a','c').by('name'); null <3>
 traversal.toString() <4>
 traversal <5> <6>
 traversal.toString() <7>

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/6e937659/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java
index 22efece..bf6bb02 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java
@@ -190,10 +190,6 @@ public final class MatchStep<S, E> extends ComputerAwareStep<S, Map<String, E>>
 
     }
 
-    public Optional<String> getStartLabel() {
-        return Optional.ofNullable(this.computedStartLabel);
-    }
-
     @Override
     public Set<String> getScopeKeys() {
         if (null == this.scopeKeys) {
@@ -331,10 +327,10 @@ public final class MatchStep<S, E> extends ComputerAwareStep<S, Map<String, E>>
                 traverser = this.starts.next();
                 final Path path = traverser.path();
                 if (!this.matchStartLabels.stream().filter(path::hasLabel).findAny().isPresent())
-                        path.addLabel(this.computedStartLabel);
+                        path.addLabel(this.computedStartLabel); // if the traverser doesn't have a legal start, then provide it the pre-computed one
                 path.addLabel(this.getId()); // so the traverser never returns to this branch ever again
             }
-
+            ///
             if (!this.isDuplicate(traverser)) {
                 if (hasMatched(this.conjunction, traverser))
                     return IteratorUtils.of(traverser.split(this.getBindings(traverser), this));
@@ -356,10 +352,10 @@ public final class MatchStep<S, E> extends ComputerAwareStep<S, Map<String, E>>
             final Traverser.Admin traverser = this.starts.next();
             final Path path = traverser.path();
             if (!this.matchStartLabels.stream().filter(path::hasLabel).findAny().isPresent())
-                path.addLabel(this.computedStartLabel);
+                path.addLabel(this.computedStartLabel); // if the traverser doesn't have a legal start, then provide it the pre-computed one
             if (!path.hasLabel(this.getId()))
                 path.addLabel(this.getId()); // so the traverser never returns to this branch ever again
-
+            ///
             if (!this.isDuplicate(traverser)) {
                 if (hasMatched(this.conjunction, traverser)) {
                     traverser.setStepId(this.getNextStep().getId());
@@ -674,10 +670,6 @@ public final class MatchStep<S, E> extends ComputerAwareStep<S, Map<String, E>>
             public final void incrementEndCount() {
                 this.multiplicity = (double) ++this.endsCount / (double) this.startsCount;
             }
-
-            /*public String toString() {
-                return this.multiplicity + "--" + this.traversal;
-            }*/
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/6e937659/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategy.java
index 56d46f1..dc26549 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategy.java
@@ -89,11 +89,12 @@ public final class MatchPredicateStrategy extends AbstractTraversalStrategy<Trav
                     break;
             }
             // match(as('a').has(key,value),...) --> as('a').has(key,value).match(...)
-            if (matchStep.getStartLabel().isPresent() && (matchStep.getPreviousStep().getLabels().isEmpty() || matchStep.getPreviousStep().getLabels().contains(matchStep.getStartLabel()))) {
+            final String startLabel = this.determineStartLabelForHasPullOut(matchStep);
+            if (null != startLabel) {
                 ((MatchStep<?, ?>) matchStep).getGlobalChildren().stream().collect(Collectors.toList()).forEach(matchTraversal -> {
                     if (matchTraversal.getStartStep() instanceof MatchStep.MatchStartStep &&
                             ((MatchStep.MatchStartStep) matchTraversal.getStartStep()).getSelectKey().isPresent() &&
-                            ((MatchStep.MatchStartStep) matchTraversal.getStartStep()).getSelectKey().get().equals(matchStep.getStartLabel().get()) &&
+                            ((MatchStep.MatchStartStep) matchTraversal.getStartStep()).getSelectKey().get().equals(startLabel) &&
                             !(matchStep.getPreviousStep() instanceof EmptyStep) &&
                             !matchTraversal.getSteps().stream()
                                     .filter(step -> !(step instanceof MatchStep.MatchStartStep) &&
@@ -104,7 +105,7 @@ public final class MatchPredicateStrategy extends AbstractTraversalStrategy<Trav
                         matchStep.removeGlobalChild(matchTraversal);
                         matchTraversal.removeStep(0);                                     // remove MatchStartStep
                         matchTraversal.removeStep(matchTraversal.getSteps().size() - 1);    // remove MatchEndStep
-                        matchStep.getPreviousStep().addLabel((String)matchStep.getStartLabel().get());
+                        matchStep.getPreviousStep().addLabel(startLabel);
                         TraversalHelper.insertTraversal(matchStep.getPreviousStep(), matchTraversal, traversal);
                     }
                 });
@@ -112,6 +113,23 @@ public final class MatchPredicateStrategy extends AbstractTraversalStrategy<Trav
         });
     }
 
+    private String determineStartLabelForHasPullOut(final MatchStep<?, ?> matchStep) {
+        if (!(matchStep.getTraversal().getParent() instanceof EmptyStep))
+            return null;
+        else {
+            final String startLabel = MatchStep.Helper.computeStartLabel(matchStep.getGlobalChildren());
+            Step<?, ?> previousStep = matchStep.getPreviousStep();
+            if(previousStep.getLabels().contains(startLabel))
+                return startLabel;
+            while (!(previousStep instanceof EmptyStep)) {
+                if (!previousStep.getLabels().isEmpty())
+                    return null;
+                previousStep = previousStep.getPreviousStep();
+            }
+            return startLabel;
+        }
+    }
+
     public static MatchPredicateStrategy instance() {
         return INSTANCE;
     }

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/6e937659/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
index 16f1f7f..eb2444c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
@@ -352,10 +352,7 @@ public final class TraversalHelper {
             if (((MatchStep.MatchStartStep) startStep).getSelectKey().isPresent())
                 variables.add(Scoping.Variable.START);
         } else if (startStep instanceof MatchStep) {
-            if (((MatchStep) startStep).getStartLabel().isPresent())
-                variables.add(Scoping.Variable.START);
-            else
-                ((MatchStep<?, ?>) startStep).getGlobalChildren().forEach(child -> TraversalHelper.getVariableLocations(variables, child));
+            ((MatchStep<?, ?>) startStep).getGlobalChildren().forEach(child -> TraversalHelper.getVariableLocations(variables, child));
         } else if (startStep instanceof ConjunctionStep || startStep instanceof NotStep || startStep instanceof WhereTraversalStep)
             ((TraversalParent) startStep).getLocalChildren().forEach(child -> TraversalHelper.getVariableLocations(variables, child));
         ///

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/6e937659/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStepTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStepTest.java
index 67da0c9..6fd1410 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStepTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStepTest.java
@@ -62,7 +62,6 @@ public class MatchStepTest extends StepTest {
         final Traversal.Admin<?, ?> traversal = __.match(as("a").out().as("b"), as("c").path().as("d")).asAdmin();
         final MatchStep<?, ?> matchStep = (MatchStep<?, ?>) traversal.getStartStep();
         assertEquals(MatchStep.class, traversal.getStartStep().getClass());
-        assertEquals("a", matchStep.getStartLabel().get());
         assertEquals(2, matchStep.getGlobalChildren().size());
         Traversal.Admin<Object, Object> pattern = matchStep.getGlobalChildren().get(0);
         assertEquals("a", ((MatchStep.MatchStartStep) pattern.getStartStep()).getSelectKey().get());
@@ -83,7 +82,6 @@ public class MatchStepTest extends StepTest {
         assertEquals(1, new HashSet<>(traversals).size()); // the two patterns should pre-compile to the same traversal
         traversals.forEach(traversal -> {
             final MatchStep<?, ?> matchStep = (MatchStep<?, ?>) traversal.getStartStep();
-            assertEquals("a", matchStep.getStartLabel().get());
             assertEquals(2, matchStep.getGlobalChildren().size());
             Traversal.Admin<Object, Object> pattern = matchStep.getGlobalChildren().get(0);
             assertEquals("a", ((MatchStep.MatchStartStep) pattern.getStartStep()).getSelectKey().get());
@@ -110,7 +108,6 @@ public class MatchStepTest extends StepTest {
         assertEquals(1, new HashSet<>(traversals).size());   // the two patterns should pre-compile to the same traversal
         traversals.forEach(traversal -> {
             MatchStep<?, ?> matchStep = (MatchStep<?, ?>) traversal.getStartStep();
-            assertEquals("a", matchStep.getStartLabel().get());
             assertEquals(2, matchStep.getGlobalChildren().size());
             Traversal.Admin<Object, Object> pattern = matchStep.getGlobalChildren().get(0);
             assertEquals("a", ((MatchStep.MatchStartStep) pattern.getStartStep()).getSelectKey().get());

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/6e937659/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategyTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategyTest.java
index 6ca4f0b..8524334 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/MatchPredicateStrategyTest.java
@@ -117,8 +117,12 @@ public class MatchPredicateStrategyTest {
         static Iterable<Object[]> generateTestParameters() {
 
             return Arrays.asList(new Traversal[][]{
-                    {__.out().match(as("a").has("name", "marko"), as("a").out().as("b")), __.out().as("a").has("name", "marko").match(as("a").out().as("b"))},
-                    // {__.match("a", as("a").out().as("b")).where(as("b").out("knows").as("c")), __.match("a", as("a").out().as("b"), as("b").where(out("knows").as("c")))},
+                    {__.out().match(as("a").has("name", "marko"), as("a").out().as("b")), __.out().as("a").has("name", "marko").match(as("a").out().as("b"))}, // has() pull out
+                    {__.out().as("a").match(as("a").has("name", "marko"), as("a").out().as("b")), __.out().as("a").has("name", "marko").match(as("a").out().as("b"))}, // has() pull out
+                    {__.out().as("a").out().match(as("a").has("name", "marko"), as("a").out().as("b")), __.out().as("a").out().match(as("a").has("name", "marko"), as("a").out().as("b"))}, // no has() pull out
+                    {__.map(__.match(as("a").has("name", "marko"), as("a").out().as("b"))), __.map(__.match(as("a").has("name", "marko"), as("a").out().as("b")))}, // no has() pull out
+                    {__.out().as("c").match(as("a").has("name", "marko"), as("a").out().as("b")), __.out().as("c").match(as("a").has("name", "marko"), as("a").out().as("b"))}, // no has() pull out
+                    //{__.match(as("a").out().as("b")).where(as("b").out("knows").as("c")), __.match(as("a").out().as("b"), as("b").where(__.out("knows").as("c")))},
             });
         }
     }