Repository: tinkerpop Updated Branches: refs/heads/TINKERPOP-1456 beaf5208d -> b5ec67506
PropertyMapStep now supports a propertyTraversal for accessing properties from the element. ConnectiveStrategy will concatenate two AND traversals if they are both filter based. SubgraphStrategy now handles valueMap() and propertyMap() correctly. Added TraversalHelper.isAllFilters() to check if a traversal is completely filter-based. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/b5ec6750 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/b5ec6750 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/b5ec6750 Branch: refs/heads/TINKERPOP-1456 Commit: b5ec675062ac3f1a7ea5f7af5ff12e51ffc94ead Parents: beaf520 Author: Marko A. Rodriguez <okramma...@gmail.com> Authored: Wed Sep 21 14:11:47 2016 -0600 Committer: Marko A. Rodriguez <okramma...@gmail.com> Committed: Wed Sep 21 14:11:47 2016 -0600 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 4 +- .../lambda/AbstractLambdaTraversal.java | 4 +- .../traversal/step/filter/ClassFilterStep.java | 15 ++- .../traversal/step/map/PropertyMapStep.java | 97 +++++++++++++++----- .../strategy/decoration/ConnectiveStrategy.java | 13 ++- .../strategy/decoration/SubgraphStrategy.java | 31 ++++--- .../process/traversal/util/TraversalHelper.java | 11 ++- .../decoration/SubgraphStrategyProcessTest.java | 9 ++ 8 files changed, 130 insertions(+), 54 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index edc479f..6233f73 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -28,8 +28,10 @@ TinkerPop 3.2.3 (Release Date: NOT OFFICIALLY RELEASED YET) * Fixed a bug in `TraversalVertexProgram` (OLAP) around ordering and connectives (i.e. `and()` and `or()`). * Added `AbstractGremlinProcessTest.checkOrderedResults()` to make testing ordered results easier. -* `AbstractLambdaTraversal` now supports a `bypassTraversal` and thus, it is possible for strategies to redefine such lambda traversals. +* `AbstractLambdaTraversal` now supports a `bypassTraversal` where it is possible for strategies to redefine such lambda traversals. * Added an internal utility `ClassFilterStep` which determines if the traverser object's class is an instance of the provided class. +* `ConnectiveStrategy` will concatenate two traversals instead of and'ing them if they are both filter-based. +* `PropertyMapStep` supports a provided traversal for accessing the properties of the element. * `SubgraphStrategy` no longer `filter()`-wraps if the criteria is a chain of filters or connectives. * `SubgraphStrategy` now supports vertex property filtering. * Fixed a bug in Gremlin-Python `P` where predicates reversed the order of the predicates. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/AbstractLambdaTraversal.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/AbstractLambdaTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/AbstractLambdaTraversal.java index b2335b9..8f910a0 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/AbstractLambdaTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/AbstractLambdaTraversal.java @@ -113,8 +113,10 @@ public abstract class AbstractLambdaTraversal<S, E> implements Traversal.Admin<S @Override public void setParent(final TraversalParent step) { - if (null != this.bypassTraversal) + if (null != this.bypassTraversal) { this.bypassTraversal.setParent(step); + step.integrateChild(this.bypassTraversal); + } } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/ClassFilterStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/ClassFilterStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/ClassFilterStep.java index 1652005..13f2aac 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/ClassFilterStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/ClassFilterStep.java @@ -29,26 +29,23 @@ import org.apache.tinkerpop.gremlin.structure.util.StringFactory; public final class ClassFilterStep<S, T> extends FilterStep<S> { private final Class<T> classFilter; - private final boolean isInstanceCheck; + private final boolean allowClasses; - public ClassFilterStep(final Traversal.Admin traversal, final Class<T> classFilter, final boolean isInstanceCheck) { + public ClassFilterStep(final Traversal.Admin traversal, final Class<T> classFilter, final boolean allowClasses) { super(traversal); this.classFilter = classFilter; - this.isInstanceCheck = isInstanceCheck; + this.allowClasses = allowClasses; } public boolean filter(final Traverser.Admin<S> traverser) { - return !(this.isInstanceCheck ? - this.classFilter.isInstance(traverser.get()) : - this.classFilter.equals(traverser.get().getClass())); - + return this.allowClasses == this.classFilter.isInstance(traverser.get()); } public int hashCode() { - return super.hashCode() ^ this.classFilter.hashCode() + Boolean.valueOf(this.isInstanceCheck).hashCode(); + return super.hashCode() ^ this.classFilter.hashCode() ^ Boolean.hashCode(this.allowClasses); } public String toString() { - return StringFactory.stepString(this, this.classFilter); + return StringFactory.stepString(this, this.classFilter.getSimpleName()); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java index 8e4e110..d10b12b 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java @@ -20,56 +20,84 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.map; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; +import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; -import org.apache.tinkerpop.gremlin.structure.*; -import org.apache.tinkerpop.gremlin.structure.util.ElementHelper; +import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; +import org.apache.tinkerpop.gremlin.structure.Element; +import org.apache.tinkerpop.gremlin.structure.Property; +import org.apache.tinkerpop.gremlin.structure.PropertyType; +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.structure.util.StringFactory; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; /** * @author Marko A. Rodriguez (http://markorodriguez.com) */ -public class PropertyMapStep<E> extends MapStep<Element, Map<String, E>> { +public class PropertyMapStep<E> extends MapStep<Element, Map<String, E>> implements TraversalParent { protected final String[] propertyKeys; protected final PropertyType returnType; protected final boolean includeTokens; + protected Traversal.Admin<Element, ? extends Property> propertyTraversal; public PropertyMapStep(final Traversal.Admin traversal, final boolean includeTokens, final PropertyType propertyType, final String... propertyKeys) { super(traversal); this.includeTokens = includeTokens; this.propertyKeys = propertyKeys; this.returnType = propertyType; + this.propertyTraversal = null; } @Override protected Map<String, E> map(final Traverser.Admin<Element> traverser) { - if (this.returnType.equals(PropertyType.VALUE)) { - final Element element = traverser.get(); - final Map map = traverser.get() instanceof Vertex ? - (Map) ElementHelper.vertexPropertyValueMap((Vertex) element, propertyKeys) : - (Map) ElementHelper.propertyValueMap(element, propertyKeys); - if (includeTokens) { - if (element instanceof VertexProperty) { - map.put(T.id, element.id()); - map.put(T.key, ((VertexProperty) element).key()); - map.put(T.value, ((VertexProperty) element).value()); - } else { - map.put(T.id, element.id()); - map.put(T.label, element.label()); + final Map<Object, Object> map = new HashMap<>(); + final Element element = traverser.get(); + final boolean isVertex = traverser.get() instanceof Vertex; + final Iterator<? extends Property> properties = null == this.propertyTraversal ? + (Iterator) element.properties(this.propertyKeys) : + TraversalUtil.applyAll(traverser, this.propertyTraversal); + while (properties.hasNext()) { + final Property property = properties.next(); + if (isVertex) { + List values = (List) map.get(property.key()); + if (null == values) { + values = new ArrayList(); + map.put(property.key(), values); } + values.add(this.returnType == PropertyType.VALUE ? property.value() : property); + } else + map.put(property.key(), this.returnType == PropertyType.VALUE ? property.value() : property); + } + if (this.returnType == PropertyType.VALUE && this.includeTokens) { + if (element instanceof VertexProperty) { + map.put(T.id, element.id()); + map.put(T.key, ((VertexProperty) element).key()); + map.put(T.value, ((VertexProperty) element).value()); + } else { + map.put(T.id, element.id()); + map.put(T.label, element.label()); } - return map; - - } else { - return traverser.get() instanceof Vertex ? - (Map) ElementHelper.vertexPropertyMap((Vertex) traverser.get(), propertyKeys) : - (Map) ElementHelper.propertyMap(traverser.get(), propertyKeys); } + return (Map) map; + } + + @Override + public List<Traversal.Admin<Element, ? extends Property>> getLocalChildren() { + return null == this.propertyTraversal ? Collections.emptyList() : Collections.singletonList(this.propertyTraversal); + } + + public void setPropertyTraversal(final Traversal.Admin<Element, ? extends Property> propertyTraversal) { + this.propertyTraversal = this.integrateChild(propertyTraversal); } public PropertyType getReturnType() { @@ -89,16 +117,35 @@ public class PropertyMapStep<E> extends MapStep<Element, Map<String, E>> { } @Override + public PropertyMapStep<E> clone() { + final PropertyMapStep<E> clone = (PropertyMapStep<E>) super.clone(); + if (null != this.propertyTraversal) + clone.propertyTraversal = this.propertyTraversal.clone(); + return clone; + } + + @Override public int hashCode() { int result = super.hashCode() ^ this.returnType.hashCode() ^ Boolean.hashCode(this.includeTokens); - for (final String propertyKey : this.propertyKeys) { - result ^= propertyKey.hashCode(); + if (null == this.propertyTraversal) { + for (final String propertyKey : this.propertyKeys) { + result ^= propertyKey.hashCode(); + } + } else { + result ^= this.propertyTraversal.hashCode(); } return result; } @Override + public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { + super.setTraversal(parentTraversal); + if (null != this.propertyTraversal) + this.integrateChild(this.propertyTraversal); + } + + @Override public Set<TraverserRequirement> getRequirements() { - return Collections.singleton(TraverserRequirement.OBJECT); + return this.getSelfAndChildRequirements(TraverserRequirement.OBJECT); } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java index 7ec1742..8627a70 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java @@ -104,10 +104,15 @@ public final class ConnectiveStrategy extends AbstractTraversalStrategy<Traversa } processConnectiveMarker(leftTraversal); - TraversalHelper.replaceStep(connectiveStep, - connectiveStep instanceof AndStep ? - new AndStep(traversal, leftTraversal, rightTraversal) : - new OrStep(traversal, leftTraversal, rightTraversal), traversal); + if (connectiveStep instanceof AndStep) { + if (TraversalHelper.filterOnlyTraversal(leftTraversal) && TraversalHelper.filterOnlyTraversal(rightTraversal)) { + TraversalHelper.insertTraversal((Step) connectiveStep, rightTraversal, traversal); + TraversalHelper.insertTraversal((Step) connectiveStep, leftTraversal, traversal); + } else + TraversalHelper.replaceStep((Step) connectiveStep, new AndStep(traversal, leftTraversal, rightTraversal), traversal); + + } else + TraversalHelper.replaceStep((Step) connectiveStep, new OrStep(traversal, leftTraversal, rightTraversal), traversal); }); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/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 5e92bc1..d34f142 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 @@ -36,6 +36,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.map.EdgeOtherVertexSt import org.apache.tinkerpop.gremlin.process.traversal.step.map.EdgeVertexStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertiesStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertyMapStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertyValueStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.VertexStep; import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; @@ -216,24 +217,30 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS // turn g.V().values() to g.V().properties().xxx.value()\ if (null != this.vertexPropertyCriterion) { final OrStep<Object> wrappedCriterion = new OrStep(traversal, - new DefaultTraversal<>().addStep(new ClassFilterStep<>(traversal, VertexProperty.class, true)), + new DefaultTraversal<>().addStep(new ClassFilterStep<>(traversal, VertexProperty.class, false)), this.vertexPropertyCriterionIsAllFilter ? this.vertexPropertyCriterion.clone() : new DefaultTraversal<>().addStep(new TraversalFilterStep<>(traversal, this.vertexPropertyCriterion.clone()))); // turn all ElementValueTraversals into filters for (final Step<?, ?> step : traversal.getSteps()) { if (step instanceof TraversalParent) { - Stream.concat(((TraversalParent) step).getGlobalChildren().stream(), ((TraversalParent) step).getLocalChildren().stream()) - .filter(t -> t instanceof ElementValueTraversal) - .forEach(t -> { - final Traversal.Admin<?, ?> temp = new DefaultTraversal<>(); - temp.addStep(new PropertiesStep<>(temp, PropertyType.PROPERTY, ((ElementValueTraversal) t).getPropertyKey())); - temp.addStep(wrappedCriterion.clone()); - temp.addStep(new PropertyValueStep<>(temp)); - temp.setParent((TraversalParent) step); - ((ElementValueTraversal) t).setBypassTraversal(temp); - ((TraversalParent) step).integrateChild(temp); - }); + if (step instanceof PropertyMapStep) { + final Traversal.Admin<?, ?> temp = new DefaultTraversal<>(); + temp.addStep(new PropertiesStep<>(temp, PropertyType.PROPERTY, ((PropertyMapStep) step).getPropertyKeys())); + temp.addStep(wrappedCriterion.clone()); + ((PropertyMapStep) step).setPropertyTraversal(temp); + } else { + Stream.concat(((TraversalParent) step).getGlobalChildren().stream(), ((TraversalParent) step).getLocalChildren().stream()) + .filter(t -> t instanceof ElementValueTraversal) + .forEach(t -> { + final Traversal.Admin<?, ?> temp = new DefaultTraversal<>(); + temp.addStep(new PropertiesStep<>(temp, PropertyType.PROPERTY, ((ElementValueTraversal) t).getPropertyKey())); + temp.addStep(wrappedCriterion.clone()); + temp.addStep(new PropertyValueStep<>(temp)); + temp.setParent((TraversalParent) step); + ((ElementValueTraversal) t).setBypassTraversal(temp); + }); + } } } for (final PropertiesStep<?> step : TraversalHelper.getStepsOfAssignableClass(PropertiesStep.class, traversal)) { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/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 14c0b5f..2000a92 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 @@ -22,8 +22,6 @@ import org.apache.tinkerpop.gremlin.process.computer.traversal.step.map.Traversa import org.apache.tinkerpop.gremlin.process.traversal.Scope; import org.apache.tinkerpop.gremlin.process.traversal.Step; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; -import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource; -import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal; import org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal; import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating; @@ -32,6 +30,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; 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.ConnectiveStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NotStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WherePredicateStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WhereTraversalStep; @@ -571,4 +570,12 @@ public final class TraversalHelper { traversal.removeStep(0); } } + + public static boolean filterOnlyTraversal(final Traversal.Admin<?, ?> traversal) { + for (final Step step : traversal.getSteps()) { + if (!(step instanceof FilterStep) && !(step instanceof ConnectiveStep)) + return false; + } + return true; + } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b5ec6750/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 c5b2d5a..fe50d20 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 @@ -28,6 +28,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSo import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; +import org.apache.tinkerpop.gremlin.structure.Column; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Vertex; import org.junit.Test; @@ -396,6 +397,14 @@ public class SubgraphStrategyProcessTest extends AbstractGremlinProcessTest { // sg = create(SubgraphStrategy.build().vertices(has("location")).vertexProperties(hasNot("endTime")).create()); checkOrderedResults(Arrays.asList("aachen", "purcellville", "santa fe", "seattle"), sg.V().order().by("location", Order.incr).values("location")); + // + sg = create(SubgraphStrategy.build().vertices(has("location")).vertexProperties(hasNot("endTime")).create()); + checkResults(Arrays.asList("aachen", "purcellville", "santa fe", "seattle"), sg.V().valueMap("location").select(Column.values).unfold().unfold()); + checkResults(Arrays.asList("aachen", "purcellville", "santa fe", "seattle"), sg.V().propertyMap("location").select(Column.values).unfold().unfold().value()); + // + sg = create(SubgraphStrategy.build().edges(__.<Edge>hasLabel("uses").has("skill", 5)).create()); + checkResults(Arrays.asList(5, 5, 5), sg.V().outE().valueMap().select(Column.values).unfold()); + checkResults(Arrays.asList(5, 5, 5), sg.V().outE().propertyMap().select(Column.values).unfold().value()); }
