found a bug in HasContainer.makeHasContainers() around AndP recurssion. No biggie, just didn't yield ultimate optimization. Found a bug in InlineFilterStrategy where hasLabel() should only back propagate into VertexStep[edges] if the step doesn't already have edge labels. Removed HasContainer.makeHasContainers() -- another ticket I'm putting into this branch. GraphTraversal.has() is fixed up accordingly with left-fold HasContainers and valid Object[] usage. Going to add a few more tests around hasXXX() steps.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/4a316c55 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/4a316c55 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/4a316c55 Branch: refs/heads/master Commit: 4a316c55c3ab1e42ef6568689cadbca0e2fb9ed8 Parents: ffe1b4c Author: Marko A. Rodriguez <okramma...@gmail.com> Authored: Tue Nov 15 08:27:24 2016 -0700 Committer: Marko A. Rodriguez <okramma...@gmail.com> Committed: Wed Nov 16 05:43:45 2016 -0700 ---------------------------------------------------------------------- .../upgrade/release-3.2.x-incubating.asciidoc | 22 +++++++++++++++ .../traversal/dsl/graph/GraphTraversal.java | 29 ++++++++++++++++---- .../traversal/step/util/HasContainer.java | 26 ++++++++++-------- .../optimization/InlineFilterStrategy.java | 20 +++++++++----- .../optimization/FilterRankingStrategyTest.java | 18 ++++++------ .../optimization/InlineFilterStrategyTest.java | 11 +++++--- .../step/sideEffect/Neo4jGraphStep.java | 11 ++++++-- .../Neo4jGraphStepStrategyTest.java | 6 ++-- .../step/sideEffect/TinkerGraphStep.java | 9 +++++- .../TinkerGraphStepStrategyTest.java | 4 +-- 10 files changed, 111 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/docs/src/upgrade/release-3.2.x-incubating.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc index 8574b88..0fd7498 100644 --- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc +++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc @@ -61,6 +61,28 @@ Upgrading for Providers Graph Database Providers ^^^^^^^^^^^^^^^^^^^^^^^^ +HasContainer AndP Splitting ++++++++++++++++++++++++++++ + +Previously, `GraphTraversal` made it easy for providers to analyze `P`-predicates in `HasContainers`, but always +splitting `AndP` predicates into their component parts. This helper behavior is no longer provided because, +1.) `AndP` can be inserted into a `XXXStep` in other ways, 2.) the providers `XXXStep` should process `AndP` +regardless of `GraphTraversal` helper, and 3.) the `GraphTraversal` helper did not recursively split. +A simple way to split `AndP` in any custom `XXXStep` that implements `HasContainerHolder` is to use the following method: + +[source,java] +---- +@Override +public void addHasContainer(final HasContainer hasContainer) { + if (hasContainer.getPredicate() instanceof AndP) { + for (final P<?> predicate : ((AndP<?>) hasContainer.getPredicate()).getPredicates()) { + this.addHasContainer(new HasContainer(hasContainer.getKey(), predicate)); + } + } else + this.hasContainers.add(hasContainer); +} +---- + Duplicate Multi-Properties ++++++++++++++++++++++++++ http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java index d8f7888..fae2d67 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java @@ -148,7 +148,6 @@ import org.apache.tinkerpop.gremlin.util.function.ConstantSupplier; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.Iterator; import java.util.List; @@ -926,8 +925,14 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { } public default GraphTraversal<S, E> has(final T accessor, final Object value) { - this.asAdmin().getBytecode().addStep(Symbols.has, accessor, value); - return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(accessor.getAccessor(), value instanceof P ? (P) value : P.eq(value))); + if (value instanceof P) + return this.has(accessor, (P) value); + else if (value instanceof Traversal) + return this.has(accessor, (Traversal) value); + else { + this.asAdmin().getBytecode().addStep(Symbols.has, accessor, value); + return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(accessor.getAccessor(), P.eq(value))); + } } public default GraphTraversal<S, E> has(final String label, final String propertyKey, final P<?> predicate) { @@ -990,7 +995,14 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { } } else ids.add(id); - Collections.addAll(ids, otherIds); + for (final Object i : otherIds) { + if (i.getClass().isArray()) { + for (final Object ii : (Object[]) i) { + ids.add(ii); + } + } else + ids.add(i); + } this.asAdmin().getBytecode().addStep(Symbols.hasId, ids.toArray()); return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.id.getAccessor(), ids.size() == 1 ? P.eq(ids.get(0)) : P.within(ids))); } @@ -1025,7 +1037,14 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { } } else values.add(value); - Collections.addAll(values, otherValues); + for (final Object v : otherValues) { + if (v.getClass().isArray()) { + for (final Object vv : (Object[]) v) { + values.add(vv); + } + } else + values.add(v); + } this.asAdmin().getBytecode().addStep(Symbols.hasValue, values.toArray()); return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.value.getAccessor(), values.size() == 1 ? P.eq(values.get(0)) : P.within(values))); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java index d08f408..3a3d9cc 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java @@ -20,7 +20,11 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.util; import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.util.AndP; -import org.apache.tinkerpop.gremlin.structure.*; +import org.apache.tinkerpop.gremlin.structure.Element; +import org.apache.tinkerpop.gremlin.structure.Property; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.apache.tinkerpop.gremlin.structure.VertexProperty; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; import java.io.Serializable; @@ -93,28 +97,23 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element> } } - protected boolean testId(Element element) - { + protected boolean testId(Element element) { return this.predicate.test(element.id()); } - protected boolean testIdAsString(Element element) - { + protected boolean testIdAsString(Element element) { return this.predicate.test(element.id().toString()); } - protected boolean testLabel(Element element) - { + protected boolean testLabel(Element element) { return this.predicate.test(element.label()); } - protected boolean testValue(Property property) - { + protected boolean testValue(Property property) { return this.predicate.test(property.value()); } - protected boolean testKey(Property property) - { + protected boolean testKey(Property property) { return this.predicate.test(property.key()); } @@ -177,6 +176,11 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element> return true; } + + /** + * @deprecated As of release 3.2.4. Providers should handle composite {@link P#and} predicates and not rely on splitting. + */ + @Deprecated public static HasContainer[] makeHasContainers(final String key, final P<?> predicate) { if (predicate instanceof AndP) { final List<P<?>> predicates = ((AndP) predicate).getPredicates(); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java index 2fa207d..17d1032 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java @@ -48,7 +48,6 @@ import org.apache.tinkerpop.gremlin.structure.T; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -73,10 +72,10 @@ public final class InlineFilterStrategy extends AbstractTraversalStrategy<Traver private static final InlineFilterStrategy INSTANCE = new InlineFilterStrategy(); private static final Set<Class<? extends OptimizationStrategy>> POSTS = new HashSet<>(Arrays.asList( - FilterRankingStrategy.class, GraphFilterStrategy.class, AdjacentToIncidentStrategy.class)); private static final Set<Class<? extends OptimizationStrategy>> PRIORS = new HashSet<>(Arrays.asList( + FilterRankingStrategy.class, IdentityRemovalStrategy.class, MatchPredicateStrategy.class)); @@ -118,18 +117,24 @@ public final class InlineFilterStrategy extends AbstractTraversalStrategy<Traver TraversalHelper.copyLabels(step, previousStep, false); traversal.removeStep(step); return true; - } else if (step.getPreviousStep() instanceof VertexStep && ((VertexStep) step.getPreviousStep()).returnsEdge()) { + } else if (step.getPreviousStep() instanceof VertexStep + && ((VertexStep) step.getPreviousStep()).returnsEdge() + && 0 == ((VertexStep) step.getPreviousStep()).getEdgeLabels().length) { final VertexStep<Edge> previousStep = (VertexStep<Edge>) step.getPreviousStep(); final List<String> edgeLabels = new ArrayList<>(); for (final HasContainer hasContainer : new ArrayList<>(step.getHasContainers())) { if (hasContainer.getKey().equals(T.label.getAccessor())) { - if (hasContainer.getBiPredicate() == Compare.eq && hasContainer.getValue() instanceof String) { + if (hasContainer.getBiPredicate() == Compare.eq && + hasContainer.getValue() instanceof String && + edgeLabels.isEmpty()) { edgeLabels.add((String) hasContainer.getValue()); step.removeHasContainer(hasContainer); - } else if (hasContainer.getBiPredicate() == Contains.within && hasContainer.getValue() instanceof Collection) { + } else if (hasContainer.getBiPredicate() == Contains.within && + hasContainer.getValue() instanceof Collection && + ((Collection) hasContainer.getValue()).containsAll(edgeLabels)) { edgeLabels.addAll((Collection<String>) hasContainer.getValue()); step.removeHasContainer(hasContainer); - } else if (hasContainer.getPredicate() instanceof OrP) { + } else if (hasContainer.getPredicate() instanceof OrP && edgeLabels.isEmpty()) { boolean removeContainer = true; final List<P<?>> orps = ((OrP) hasContainer.getPredicate()).getPredicates(); final List<String> newEdges = new ArrayList<>(); @@ -266,11 +271,12 @@ public final class InlineFilterStrategy extends AbstractTraversalStrategy<Traver matchTraversal.getStartStep() instanceof MatchStep.MatchStartStep && startLabel.equals(((MatchStep.MatchStartStep) matchTraversal.getStartStep()).getSelectKey().orElse(null))) { changed = true; + step.removeGlobalChild(matchTraversal); final String endLabel = ((MatchStep.MatchEndStep) matchTraversal.getEndStep()).getMatchKey().orElse(null); // why would this exist? but just in case matchTraversal.removeStep(0); // remove MatchStartStep matchTraversal.removeStep(matchTraversal.getSteps().size() - 1); // remove MatchEndStep TraversalHelper.applySingleLevelStrategies(traversal, matchTraversal, InlineFilterStrategy.class); - step.removeGlobalChild(matchTraversal); + matchTraversal.getEndStep().addLabel(startLabel); if (null != endLabel) matchTraversal.getEndStep().addLabel(endLabel); TraversalHelper.insertTraversal((Step) step.getPreviousStep(), matchTraversal, traversal); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java index ff72f8c..fbeeef5 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java @@ -84,13 +84,13 @@ public class FilterRankingStrategyTest { {__.dedup().order(), __.dedup().order(), Collections.emptyList()}, {__.has("name", "marko").as("a").out().as("b").has("age", 32).where("a", neq("b")).as("c").out(), __.has("name", "marko").as("a").out().has("age", 32).as("b").where("a", neq("b")).as("c").out(), Collections.emptyList()}, {__.has("name", "marko").as("a").out().has("age", 32).as("b").where("a", neq("b")), __.has("name", "marko").as("a").out().has("age", 32).as("b").where("a", neq("b")), Collections.emptyList()}, - {__.has("name", "marko").has("age", 32).dedup().has("name", "bob").as("a"), __.has("name", "marko").has("age", 32).has("name", "bob").dedup().as("a"), Collections.emptyList()}, - {__.has("name", "marko").dedup().as("a").has("age", 32).has("name", "bob").as("b"), __.has("name", "marko").has("age", 32).has("name", "bob").dedup().as("b", "a"), Collections.emptyList()}, + {__.has("name", "marko").has("age", 32).dedup().has("name", "bob").as("a"), __.has("name", "marko").has("age", 32).has("name", "bob").dedup().as("a"), Collections.singletonList(InlineFilterStrategy.instance())}, + {__.has("name", "marko").dedup().as("a").has("age", 32).has("name", "bob").as("b"), __.has("name", "marko").has("age", 32).has("name", "bob").dedup().as("b", "a"), Collections.singletonList(InlineFilterStrategy.instance())}, {__.where("b", eq("c")).as("a").dedup("a").has("name", "marko"), __.has("name", "marko").where("b", eq("c")).as("a").dedup("a"), Collections.emptyList()}, - {__.where("b", eq("c")).has("name", "bob").as("a").dedup("a").has("name", "marko"), __.has("name", "bob").has("name", "marko").where("b", eq("c")).as("a").dedup("a"), Collections.emptyList()}, - {__.has("name","marko").as("a").out().has("name","bob").dedup().as("b").where(__.as("a").out().as("b")),__.has("name","marko").as("a").out().has("name","bob").dedup().as("b").where(__.as("a").out().as("b")),Collections.emptyList()}, - {__.has("name","marko").as("a").out().has("name","bob").as("b").dedup().where(__.as("a").out().as("b")),__.has("name","marko").as("a").out().has("name","bob").dedup().as("b").where(__.as("a").out().as("b")),Collections.emptyList()}, - {__.has("name","marko").as("a").out().has("name","bob").dedup().as("c").where(__.as("a").out().as("b")),__.has("name","marko").as("a").out().has("name","bob").where(__.as("a").out().as("b")).dedup().as("c"),Collections.emptyList()}, + {__.where("b", eq("c")).has("name", "bob").as("a").dedup("a").has("name", "marko"), __.has("name", "bob").has("name", "marko").where("b", eq("c")).as("a").dedup("a"), Collections.singletonList(InlineFilterStrategy.instance())}, + {__.has("name", "marko").as("a").out().has("name", "bob").dedup().as("b").where(__.as("a").out().as("b")), __.has("name", "marko").as("a").out().has("name", "bob").dedup().as("b").where(__.as("a").out().as("b")), Collections.singletonList(InlineFilterStrategy.instance())}, + {__.has("name", "marko").as("a").out().has("name", "bob").as("b").dedup().where(__.as("a").out().as("b")), __.has("name", "marko").as("a").out().has("name", "bob").dedup().as("b").where(__.as("a").out().as("b")), Collections.singletonList(InlineFilterStrategy.instance())}, + {__.has("name", "marko").as("a").out().has("name", "bob").dedup().as("c").where(__.as("a").out().as("b")), __.has("name", "marko").as("a").out().has("name", "bob").where(__.as("a").out().as("b")).dedup().as("c"), Collections.singletonList(InlineFilterStrategy.instance())}, {__.order().dedup(), __.dedup().order(), Collections.emptyList()}, {__.order().filter(testP).dedup(), __.order().filter(testP).dedup(), Collections.emptyList()}, {__.order().as("a").dedup(), __.dedup().order().as("a"), Collections.emptyList()}, @@ -102,13 +102,13 @@ public class FilterRankingStrategyTest { {__.order().identity().dedup(), __.dedup().order(), Collections.singletonList(IdentityRemovalStrategy.instance())}, {__.order().out().dedup(), __.order().out().dedup(), Collections.emptyList()}, {has("value", 0).filter(out()).dedup(), has("value", 0).filter(out()).dedup(), Collections.emptyList()}, - {__.dedup().has("value", 0).or(not(has("age")), has("age", 10)).has("value", 1), __.has("value", 0).has("value", 1).or(not(has("age")), has("age", 10)).dedup(), Collections.emptyList()}, + {__.dedup().has("value", 0).or(not(has("age")), has("age", 10)).has("value", 1), __.has("value", 0).has("value", 1).or(not(has("age")), has("age", 10)).dedup(), Collections.singletonList(InlineFilterStrategy.instance())}, {__.dedup().filter(out()).has("value", 0), has("value", 0).filter(out()).dedup(), Collections.emptyList()}, {filter(out()).dedup().has("value", 0), has("value", 0).filter(out()).dedup(), Collections.emptyList()}, {has("value", 0).filter(out()).dedup(), has("value", 0).filter(out()).dedup(), Collections.emptyList()}, - {has("value", 0).or(has("name"), has("age")).has("value", 1).dedup(), has("value", 0).has("value", 1).or(has("name"), has("age")).dedup(), Collections.emptyList()}, + {has("value", 0).or(has("name"), has("age")).has("value", 1).dedup(), has("value", 0).has("value", 1).or(has("name"), has("age")).dedup(), Collections.singletonList(InlineFilterStrategy.instance())}, {has("value", 0).or(out(), in()).as(Graph.Hidden.hide("x")).has("value", 1).dedup(), has("value", 0).has("value", 1).or(outE(), inE()).dedup(), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, - {has("value", 0).and(has("age"), has("name", "marko")).is(10), __.is(10).has("value", 0).has("name", "marko").has("age"), Collections.singletonList(InlineFilterStrategy.instance())}, + // {has("value", 0).and(has("age"), has("name", "marko")).is(10), __.is(10).has("value", 0).has("name", "marko").has("age"), Collections.singletonList(InlineFilterStrategy.instance())}, {has("value", 0).filter(or(not(has("age")), has("age", 1))).has("value", 1).dedup(), has("value", 0).has("value", 1).or(not(filter(properties("age"))), has("age", 1)).dedup(), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, }); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java index c2dbcc4..0d02fe4 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java @@ -110,7 +110,10 @@ public class InlineFilterStrategyTest { {match(as("a").has("age", 10).both().as("b"), as("b").out().as("c")), match(as("a").has("age", 10).both().as("b"), as("b").out().as("c"))}, {__.map(match(as("a").has("age", 10), as("a").filter(has("name")).as("b"))), __.map(match(as("a").has("age", 10), as("a").has("name").as("b")))}, {V().match(as("a").has("age", 10)), V().has("age", 10).as("a")}, - {V().match(as("a").has("age", 10).as("b"), as("a").filter(has("name")).as("b")), V().has("age", 10).as("a","b").match(as("a").has("name").as("b"))}, + {V().match(as("a").has("age", 10).has("name", "marko").as("b")), V().has("age", 10).has("name", "marko").as("a", "b")}, + {V().match(as("a").has("age", 10).has("name", "marko").as("b"), as("a").out("knows").as("c")), V().has("age", 10).has("name", "marko").as("a", "b").match(as("a").out("knows").as("c"))}, + {V().match(as("a").out("knows").as("c"), as("a").has("age", 10).has("name", "marko").as("b")), V().has("age", 10).has("name", "marko").as("a", "b").match(as("a").out("knows").as("c"))}, + {V().match(as("a").has("age", 10).as("b"), as("a").filter(has("name")).as("b")), V().has("age", 10).as("a", "b").match(as("a").has("name").as("b"))}, // {filter(dedup()), filter(dedup())}, {filter(filter(drop())), filter(drop())}, @@ -118,9 +121,9 @@ public class InlineFilterStrategyTest { {filter(tail(10).as("a")), filter(tail(10).as("a"))}, // {outE().hasLabel("knows").inV(), outE("knows").inV()}, - {outE().hasLabel("knows").hasLabel("created").inV(), outE("created").inV()}, - {outE().or(hasLabel("knows"),hasLabel("created")).inV(), outE("knows", "created").inV()}, - {outE().or(hasLabel("knows").as("a"),hasLabel("created").as("b")).as("c").inV(), outE("knows", "created").as("a","b","c").inV()}, + {outE().hasLabel("knows").hasLabel("created").inV(), outE("knows").hasLabel("created").inV()}, + {outE().or(hasLabel("knows"), hasLabel("created")).inV(), outE("knows", "created").inV()}, + {outE().or(hasLabel("knows").as("a"), hasLabel("created").as("b")).as("c").inV(), outE("knows", "created").as("a", "b", "c").inV()}, {outE().hasLabel(P.eq("knows").or(P.gt("created"))).has("weight", gt(1.0)).inV(), addHas(outE(), T.label.getAccessor(), P.eq("knows").or(P.gt("created")), "weight", gt(1.0)).inV()}, {outE().hasLabel(P.eq("knows").or(P.eq("created"))).has("weight", gt(1.0)).inV(), outE("knows", "created").has("weight", gt(1.0)).inV()}, // {outE().or(has(T.label,P.within("knows","likes")).hasLabel("created")).inV(), outE("knows", "likes", "created").inV()}, http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java ---------------------------------------------------------------------- diff --git a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java index 6b4e9c4..7df47d0 100644 --- a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java +++ b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java @@ -19,9 +19,11 @@ package org.apache.tinkerpop.gremlin.neo4j.process.traversal.step.sideEffect; import org.apache.tinkerpop.gremlin.neo4j.structure.Neo4jGraph; +import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; +import org.apache.tinkerpop.gremlin.process.traversal.util.AndP; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Element; import org.apache.tinkerpop.gremlin.structure.Vertex; @@ -50,7 +52,7 @@ public final class Neo4jGraphStep<S, E extends Element> extends GraphStep<S, E> } private Iterator<? extends Edge> edges() { - return IteratorUtils.filter(this.getTraversal().getGraph().get().edges(this.ids), edge -> HasContainer.testAll((Edge) edge, this.hasContainers)); + return IteratorUtils.filter(this.getTraversal().getGraph().get().edges(this.ids), edge -> HasContainer.testAll(edge, this.hasContainers)); } private Iterator<? extends Vertex> vertices() { @@ -75,7 +77,12 @@ public final class Neo4jGraphStep<S, E extends Element> extends GraphStep<S, E> @Override public void addHasContainer(final HasContainer hasContainer) { - this.hasContainers.add(hasContainer); + if (hasContainer.getPredicate() instanceof AndP) { + for (final P<?> predicate : ((AndP<?>) hasContainer.getPredicate()).getPredicates()) { + this.addHasContainer(new HasContainer(hasContainer.getKey(), predicate)); + } + } else + this.hasContainers.add(hasContainer); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java ---------------------------------------------------------------------- diff --git a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java b/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java index 3f8cd02..a6b0862 100644 --- a/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java +++ b/neo4j-gremlin/src/test/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/strategy/optimization/Neo4jGraphStepStrategyTest.java @@ -18,7 +18,6 @@ */ package org.apache.tinkerpop.gremlin.neo4j.process.traversal.strategy.optimization; -import org.apache.tinkerpop.gremlin.neo4j.AbstractNeo4jGremlinTest; import org.apache.tinkerpop.gremlin.neo4j.process.traversal.step.sideEffect.Neo4jGraphStep; import org.apache.tinkerpop.gremlin.neo4j.structure.Neo4jGraph; import org.apache.tinkerpop.gremlin.process.traversal.P; @@ -28,7 +27,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.DefaultGraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; -import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.FilterRankingStrategy; @@ -120,9 +118,9 @@ public class Neo4jGraphStepStrategyTest { {__.V().as("a").has("name", "marko").as("b").or(has("age"), has("age", gt(32))).has("lang", "java"), g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))).as("b", "a"), Collections.singletonList(FilterRankingStrategy.instance())}, {__.V().as("a").dedup().has("name", "marko").or(has("age"), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), - g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(has("age"), has("age", gt(32))).dedup().as("a"), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance())}, + g_V("name", eq("marko"), "lang", eq("java"), "name", eq("bob")).or(has("age"), has("age", gt(32))).dedup().as("a"), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance())}, {__.V().as("a").dedup().has("name", "marko").or(has("age", 10), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), - g_V("name", eq("marko"), "age", eq(10).or(gt(32)), "name", eq("bob"), "lang", eq("java")).dedup().as("a"), TraversalStrategies.GlobalCache.getStrategies(Neo4jGraph.class).toList()}, + g_V("name", eq("marko"), "lang", eq("java"), "name", eq("bob"), "age", eq(10).or(gt(32))).dedup().as("a"), TraversalStrategies.GlobalCache.getStrategies(Neo4jGraph.class).toList()}, {__.V().has("name", "marko").or(not(has("age")), has("age", gt(32))).has("name", "bob").has("lang", "java"), g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(not(filter(properties("age"))), has("age", gt(32))), TraversalStrategies.GlobalCache.getStrategies(Neo4jGraph.class).toList()}, /////// http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java index 23ba817..36c67ac 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java @@ -19,9 +19,11 @@ package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.sideEffect; import org.apache.tinkerpop.gremlin.process.traversal.Compare; +import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; +import org.apache.tinkerpop.gremlin.process.traversal.util.AndP; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Element; import org.apache.tinkerpop.gremlin.structure.Vertex; @@ -120,7 +122,12 @@ public final class TinkerGraphStep<S, E extends Element> extends GraphStep<S, E> @Override public void addHasContainer(final HasContainer hasContainer) { - this.hasContainers.add(hasContainer); + if (hasContainer.getPredicate() instanceof AndP) { + for (final P<?> predicate : ((AndP<?>) hasContainer.getPredicate()).getPredicates()) { + this.addHasContainer(new HasContainer(hasContainer.getKey(), predicate)); + } + } else + this.hasContainers.add(hasContainer); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4a316c55/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java index c54a6b7..393cf32 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java @@ -119,9 +119,9 @@ public class TinkerGraphStepStrategyTest { {__.V().as("a").has("name", "marko").as("b").or(has("age"), has("age", gt(32))).has("lang", "java"), g_V("name", eq("marko"), "lang", eq("java")).or(has("age"), has("age", gt(32))).as("b", "a"), Collections.singletonList(FilterRankingStrategy.instance())}, {__.V().as("a").dedup().has("name", "marko").or(has("age"), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), - g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(has("age"), has("age", gt(32))).dedup().as("a"), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance())}, + g_V("name", eq("marko"), "lang", eq("java"), "name", eq("bob")).or(has("age"), has("age", gt(32))).dedup().as("a"), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance())}, {__.V().as("a").dedup().has("name", "marko").or(has("age", 10), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), - g_V("name", eq("marko"), "age", eq(10).or(gt(32)), "name", eq("bob"), "lang", eq("java")).dedup().as("a"), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + g_V("name", eq("marko"), "lang", eq("java"), "name", eq("bob"), "age", eq(10).or(gt(32))).dedup().as("a"), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, {__.V().has("name", "marko").or(not(has("age")), has("age", gt(32))).has("name", "bob").has("lang", "java"), g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(not(filter(properties("age"))), has("age", gt(32))), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, ///////
