Repository: tinkerpop Updated Branches: refs/heads/TINKERPOP-1777 be3a5db18 -> 087525e24 (forced update)
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/d1121544 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/d1121544 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/d1121544 Branch: refs/heads/TINKERPOP-1777 Commit: d1121544017acf1189f0270f60b5f1f402fec0ea 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:24:54 2018 -0500 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../strategy/decoration/SubgraphStrategy.java | 58 ++++++++++---- .../decoration/SubgraphStrategyProcessTest.java | 84 ++++++++++++++++++++ 3 files changed, 127 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/d1121544/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f1519b6..1701342 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -23,6 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima [[release-3-2-8]] === TinkerPop 3.2.8 (Release Date: NOT OFFICIALLY RELEASED YET) +* Added `checkAdjacentVertices` option to `SubgraphStrategy`. * Modified `GremlinDslProcessor` so that it generated the `getAnonymousTraversalClass()` method to return the DSL version of `__`. * Added the "Kitchen Sink" test data set. * Fixed a bug in `NumberHelper` that led to wrong min/max results if numbers exceeded the Integer limits. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/d1121544/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/d1121544/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"));
