You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2018/02/15 21:24:02 UTC

tinkerpop git commit: TINKERPOP-1586 Added checkAdjacentVertices option to SubgraphStrategy

Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1586 [created] a9418d75c


TINKERPOP-1586 Added checkAdjacentVertices option to SubgraphStrategy

This change allows the user to turn off an aspect of SubgraphStrategy that prevents it from working properly in OLAP situations.


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

Branch: refs/heads/TINKERPOP-1586
Commit: a9418d75cce5d3be1c43dbc5ca44df0ee3e0994f
Parents: bcffaad
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Feb 15 16:22:58 2018 -0500
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Feb 15 16:22:58 2018 -0500

----------------------------------------------------------------------
 .../strategy/decoration/SubgraphStrategy.java   | 58 ++++++++++----
 .../decoration/SubgraphStrategyProcessTest.java | 84 ++++++++++++++++++++
 2 files changed, 126 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a9418d75/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
index 4747cd4..e0d260f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
@@ -82,13 +82,14 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
 
     private final String MARKER = Graph.Hidden.hide("gremlin.subgraphStrategy");
 
-    private SubgraphStrategy(final Traversal<Vertex, ?> vertexCriterion, final Traversal<Edge, ?> edgeCriterion, final Traversal<VertexProperty, ?> vertexPropertyCriterion) {
+    private SubgraphStrategy(final Builder builder) {
 
-        this.vertexCriterion = null == vertexCriterion ? null : vertexCriterion.asAdmin().clone();
+        this.vertexCriterion = null == builder.vertexCriterion ? null : builder.vertexCriterion.asAdmin().clone();
 
-        // if there is no vertex predicate there is no need to test either side of the edge
-        if (null == this.vertexCriterion) {
-            this.edgeCriterion = null == edgeCriterion ? null : edgeCriterion.asAdmin().clone();
+        // if there is no vertex predicate there is no need to test either side of the edge - also this option can
+        // be simply configured in the builder to not be used
+        if (null == this.vertexCriterion || !builder.checkAdjacentVertices) {
+            this.edgeCriterion = null == builder.edgeCriterion ? null : builder.edgeCriterion.asAdmin().clone();
         } else {
             final Traversal.Admin<Edge, ?> vertexPredicate;
             vertexPredicate = __.<Edge>and(
@@ -97,12 +98,12 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
 
             // if there is a vertex predicate then there is an implied edge filter on vertices even if there is no
             // edge predicate provided by the user.
-            this.edgeCriterion = null == edgeCriterion ?
+            this.edgeCriterion = null == builder.edgeCriterion ?
                     vertexPredicate :
-                    edgeCriterion.asAdmin().clone().addStep(new TraversalFilterStep<>(edgeCriterion.asAdmin(), vertexPredicate));
+                    builder.edgeCriterion.asAdmin().clone().addStep(new TraversalFilterStep<>(builder.edgeCriterion.asAdmin(), vertexPredicate));
         }
 
-        this.vertexPropertyCriterion = null == vertexPropertyCriterion ? null : vertexPropertyCriterion.asAdmin().clone();
+        this.vertexPropertyCriterion = null == builder.vertexPropertyCriterion ? null : builder.vertexPropertyCriterion.asAdmin().clone();
 
         if (null != this.vertexCriterion)
             TraversalHelper.applyTraversalRecursively(t -> t.getStartStep().addLabel(MARKER), this.vertexCriterion);
@@ -316,25 +317,50 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
 
     public final static class Builder {
 
-        private Traversal<Vertex, ?> vertexPredicate = null;
-        private Traversal<Edge, ?> edgePredicate = null;
-        private Traversal<VertexProperty, ?> vertexPropertyPredicate = null;
+        private Traversal<Vertex, ?> vertexCriterion = null;
+        private Traversal<Edge, ?> edgeCriterion = null;
+        private Traversal<VertexProperty, ?> vertexPropertyCriterion = null;
+        private boolean checkAdjacentVertices = true;
 
         private Builder() {
         }
 
+        /**
+         * Enables the strategy to apply the {@link #vertices(Traversal)} filter to the adjacent vertices of an edge.
+         * If using this strategy for OLAP then this value should be set to {@code false} as checking adjacent vertices
+         * will force the traversal to leave the local star graph (which is not possible in OLAP) and will cause an
+         * error. By default, this value is {@code true}.
+         */
+        public Builder checkAdjacentVertices(final boolean enable) {
+            this.checkAdjacentVertices = enable;
+            return this;
+        }
+
+        /**
+         * The traversal predicate that defines the vertices to include in the subgraph. If
+         * {@link #checkAdjacentVertices(boolean)} is {@code true} then this predicate will also be applied to the
+         * adjacent vertices of edges. Take care when setting this value for OLAP based traversals as the traversal
+         * predicate cannot be written in such a way as to leave the local star graph and can thus only evaluate the
+         * current vertex and its related edges.
+         */
         public Builder vertices(final Traversal<Vertex, ?> vertexPredicate) {
-            this.vertexPredicate = vertexPredicate;
+            this.vertexCriterion = vertexPredicate;
             return this;
         }
 
+        /**
+         * The traversal predicate that defines the edges to include in the subgraph.
+         */
         public Builder edges(final Traversal<Edge, ?> edgePredicate) {
-            this.edgePredicate = edgePredicate;
+            this.edgeCriterion = edgePredicate;
             return this;
         }
 
+        /**
+         * The traversal predicate that defines the vertex properties to include in the subgraph.
+         */
         public Builder vertexProperties(final Traversal<VertexProperty, ?> vertexPropertyPredicate) {
-            this.vertexPropertyPredicate = vertexPropertyPredicate;
+            this.vertexPropertyCriterion = vertexPropertyPredicate;
             return this;
         }
 
@@ -355,9 +381,9 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
         }
 
         public SubgraphStrategy create() {
-            if (null == this.vertexPredicate && null == this.edgePredicate && null == this.vertexPropertyPredicate)
+            if (null == this.vertexCriterion && null == this.edgeCriterion && null == this.vertexPropertyCriterion)
                 throw new IllegalStateException("A subgraph must be filtered by a vertex, edge, or vertex property criterion");
-            return new SubgraphStrategy(this.vertexPredicate, this.edgePredicate, this.vertexPropertyPredicate);
+            return new SubgraphStrategy(this);
         }
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a9418d75/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java
index ad2501d..2565770 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java
@@ -252,6 +252,90 @@ public class SubgraphStrategyProcessTest extends AbstractGremlinProcessTest {
 
     @Test
     @LoadGraphWith(MODERN)
+    public void shouldFilterMixedCriteriaButNotCheckAdjacentVertices() {
+        final Traversal<Vertex, ?> vertexCriterion = has("name", P.within("josh", "lop", "ripple"));
+
+        // 9 isn't present because marko is not in the vertex list
+        final Traversal<Edge, ?> edgeCriterion = __.or(
+                has("weight", 0.4d).hasLabel("created"), // 11
+                has("weight", 1.0d).hasLabel("created") // 10
+        );
+
+        final SubgraphStrategy strategy = SubgraphStrategy.build().
+                checkAdjacentVertices(false).
+                edges(edgeCriterion).vertices(vertexCriterion).create();
+        final GraphTraversalSource sg = g.withStrategies(strategy);
+
+        // three vertices are included in the subgraph
+        assertEquals(6, g.V().count().next().longValue());
+        assertEquals(3, sg.V().count().next().longValue());
+
+        // three edges are explicitly included as we ignore checking of adjacent vertices
+        assertEquals(6, g.E().count().next().longValue());
+        assertEquals(3, sg.E().count().next().longValue());
+
+        // from vertex
+
+        assertEquals(2, g.V(convertToVertexId("josh")).outE().count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).outE().count().next().longValue());
+        assertEquals(2, g.V(convertToVertexId("josh")).out().count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).out().count().next().longValue());
+
+        assertEquals(1, g.V(convertToVertexId("josh")).inE().count().next().longValue());
+        assertEquals(0, sg.V(convertToVertexId("josh")).inE().count().next().longValue());
+        assertEquals(1, g.V(convertToVertexId("josh")).in().count().next().longValue());
+        assertEquals(0, sg.V(convertToVertexId("josh")).in().count().next().longValue());
+
+        assertEquals(3, g.V(convertToVertexId("josh")).bothE().count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).bothE().count().next().longValue());
+        assertEquals(3, g.V(convertToVertexId("josh")).both().count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).both().count().next().longValue());
+
+        // marko not present directly because of vertexCriterion - only accessible via vertices in the subgraph
+        assertEquals(1, g.V(convertToVertexId("marko")).count().next().longValue());
+        assertEquals(0, sg.V(convertToVertexId("marko")).count().next().longValue());
+
+        // with label
+
+        assertEquals(2, g.V(convertToVertexId("josh")).outE("created").count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).outE("created").count().next().longValue());
+        assertEquals(2, g.V(convertToVertexId("josh")).out("created").count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).out("created").count().next().longValue());
+        assertEquals(2, g.V(convertToVertexId("josh")).bothE("created").count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).bothE("created").count().next().longValue());
+        assertEquals(2, g.V(convertToVertexId("josh")).both("created").count().next().longValue());
+        assertEquals(2, sg.V(convertToVertexId("josh")).both("created").count().next().longValue());
+
+        assertEquals(1, g.V(convertToVertexId("josh")).inE("knows").count().next().longValue());
+        assertEquals(0, sg.V(convertToVertexId("josh")).inE("knows").count().next().longValue());
+        assertEquals(1, g.V(convertToVertexId("josh")).in("knows").count().next().longValue());
+        assertEquals(0, sg.V(convertToVertexId("josh")).in("knows").count().next().longValue());
+        assertEquals(1, g.V(convertToVertexId("josh")).bothE("knows").count().next().longValue());
+        assertEquals(0, sg.V(convertToVertexId("josh")).bothE("knows").count().next().longValue());
+        assertEquals(1, g.V(convertToVertexId("josh")).both("knows").count().next().longValue());
+        assertEquals(0, sg.V(convertToVertexId("josh")).both("knows").count().next().longValue());
+
+        // with branch factor
+
+        assertEquals(1, g.V(convertToVertexId("josh")).local(bothE().limit(1)).count().next().longValue());
+        assertEquals(1, sg.V(convertToVertexId("josh")).local(bothE().limit(1)).count().next().longValue());
+        assertEquals(1, g.V(convertToVertexId("josh")).local(bothE().limit(1)).inV().count().next().longValue());
+        assertEquals(1, sg.V(convertToVertexId("josh")).local(bothE().limit(1)).inV().count().next().longValue());
+        assertEquals(1, g.V(convertToVertexId("josh")).local(bothE("knows", "created").limit(1)).count().next().longValue());
+        assertEquals(1, sg.V(convertToVertexId("josh")).local(bothE("knows", "created").limit(1)).count().next().longValue());
+        assertEquals(1, g.V(convertToVertexId("josh")).local(bothE("knows", "created").limit(1)).inV().count().next().longValue());
+        assertEquals(1, sg.V(convertToVertexId("josh")).local(bothE("knows", "created").limit(1)).inV().count().next().longValue());
+
+        // from edge
+
+        // marko is not accessible from the edge
+        assertEquals(2, g.E(convertToEdgeId("marko", "created", "lop")).bothV().count().next().longValue());
+        assertEquals(1, sg.E(convertToEdgeId("marko", "created", "lop")).bothV().count().next().longValue());
+    }
+
+
+    @Test
+    @LoadGraphWith(MODERN)
     public void shouldFilterMixedCriteria() throws Exception {
         final Traversal<Vertex, ?> vertexCriterion = has("name", P.within("josh", "lop", "ripple"));