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 2017/01/09 16:36:25 UTC
tinkerpop git commit: fixed an NPE in AddVertexStartStep. In
PathRetractionStrategy,
NoOpBarriers are only added to global children as 99% of local children are
by()-modulation based and thus,
there is no point to fully compute the traversal if only the
Repository: tinkerpop
Updated Branches:
refs/heads/TINKERPOP-1521 [created] 276064530
fixed an NPE in AddVertexStartStep. In PathRetractionStrategy, NoOpBarriers are only added to global children as 99% of local children are by()-modulation based and thus, there is no point to fully compute the traversal if only the first next() is going to be used.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/27606453
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/27606453
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/27606453
Branch: refs/heads/TINKERPOP-1521
Commit: 276064530617c04706b62ac57ec4e2e660a756a4
Parents: 2d824cf
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Mon Jan 9 09:36:15 2017 -0700
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Mon Jan 9 09:36:15 2017 -0700
----------------------------------------------------------------------
CHANGELOG.asciidoc | 2 +
.../traversal/step/map/AddVertexStartStep.java | 8 +--
.../optimization/PathRetractionStrategy.java | 3 +-
.../step/map/GroovyAddVertexTest.groovy | 10 ++++
.../traversal/step/map/AddVertexTest.java | 53 +++++++++++++++++++-
5 files changed, 70 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/27606453/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 805db7b..4411f99 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* `PathRetractionStrategy` does not add a `NoOpBarrierStep` to the end of local children as its wasted computation in 99% of traversals.
+* Fixed a bug in `AddVertexStartStep` where if a side-effect was being used in the parametrization, an NPE occurred.
* Fixed a bug in `LazyBarrierStrategy` where `profile()` was deactivating it accidentally.
* Fixed a bug in `RepeatUnrollStrategy` where stateful `DedupGlobalStep` was cloned and thus, maintained two deduplication sets.
* Added documentation around "terminal steps" in Gremlin: `hasNext()`, `next()`, `toList()`, etc.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/27606453/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
index 24fedd2..58c3ef3 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
@@ -19,8 +19,10 @@
package org.apache.tinkerpop.gremlin.process.traversal.step.map;
import org.apache.tinkerpop.gremlin.process.traversal.Parameterizing;
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.TraverserGenerator;
import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep;
@@ -29,7 +31,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRe
import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.ListCallbackRegistry;
import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
-import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.EmptyTraverser;
import org.apache.tinkerpop.gremlin.process.traversal.util.FastNoSuchElementException;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
@@ -75,12 +76,13 @@ public final class AddVertexStartStep extends AbstractStep<Vertex, Vertex> imple
protected Traverser.Admin<Vertex> processNextStart() {
if (this.first) {
this.first = false;
- final Vertex vertex = this.getTraversal().getGraph().get().addVertex(this.parameters.getKeyValues(EmptyTraverser.instance()));
+ final TraverserGenerator generator = this.getTraversal().getTraverserGenerator();
+ final Vertex vertex = this.getTraversal().getGraph().get().addVertex(this.parameters.getKeyValues(generator.generate(false, (Step) this, 1L)));
if (this.callbackRegistry != null) {
final Event.VertexAddedEvent vae = new Event.VertexAddedEvent(DetachedFactory.detach(vertex, true));
this.callbackRegistry.getCallbacks().forEach(c -> c.accept(vae));
}
- return this.getTraversal().getTraverserGenerator().generate(vertex, this, 1l);
+ return generator.generate(vertex, this, 1L);
} else
throw FastNoSuchElementException.instance();
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/27606453/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
index da869b2..1d09748 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java
@@ -109,7 +109,8 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav
!(currentStep instanceof MatchStep) &&
!(currentStep instanceof Barrier) &&
!(currentStep.getNextStep() instanceof Barrier) &&
- !(currentStep.getTraversal().getParent() instanceof MatchStep))
+ !(currentStep.getTraversal().getParent() instanceof MatchStep) &&
+ TraversalHelper.isGlobalChild(currentStep.getTraversal()))
TraversalHelper.insertAfterStep(new NoOpBarrierStep<>(traversal, this.standardBarrierSize), currentStep, traversal);
}
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/27606453/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyAddVertexTest.groovy
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyAddVertexTest.groovy b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyAddVertexTest.groovy
index 5ff3fb3..00312fa 100644
--- a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyAddVertexTest.groovy
+++ b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroovyAddVertexTest.groovy
@@ -76,6 +76,16 @@ public abstract class GroovyAddVertexTest {
new ScriptTraversal<>(g, "gremlin-groovy", "g.V.addV('animal').property('name', values('name')).property('name', 'an animal').property(values('name'), label())")
}
+ @Override
+ public Traversal<Vertex, Map<String, List<String>>> get_g_withSideEffectXa_testX_V_hasLabelXsoftwareX_propertyXtemp_selectXaXX_valueMapXname_tempX() {
+ new ScriptTraversal<>(g, "gremlin-groovy", "g.withSideEffect('a', 'test').V.hasLabel('software').property('temp', select('a')).valueMap('name', 'temp')")
+ }
+
+ @Override
+ public Traversal<Vertex, String> get_g_withSideEffectXa_markoX_addV_propertyXname_selectXaXX_name() {
+ new ScriptTraversal<>(g, "gremlin-groovy", "g.withSideEffect('a', 'marko').addV().property('name', select('a')).name")
+ }
+
///////// DEPRECATED BELOW
@Override
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/27606453/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java
index 0186fa6..f43c612 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java
@@ -32,11 +32,17 @@ import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
+import java.util.Collections;
import java.util.List;
+import java.util.Map;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.select;
import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
@@ -62,6 +68,10 @@ public abstract class AddVertexTest extends AbstractGremlinTest {
public abstract Traversal<Vertex, Vertex> get_g_V_addVXanimalX_propertyXname_valuesXnameXX_propertyXname_an_animalX_propertyXvaluesXnameX_labelX();
+ public abstract Traversal<Vertex, Map<String, List<String>>> get_g_withSideEffectXa_testX_V_hasLabelXsoftwareX_propertyXtemp_selectXaXX_valueMapXname_tempX();
+
+ public abstract Traversal<Vertex, String> get_g_withSideEffectXa_markoX_addV_propertyXname_selectXaXX_name();
+
// 3.0.0 DEPRECATIONS
@Deprecated
public abstract Traversal<Vertex, Vertex> get_g_V_addVXlabel_animal_age_0X();
@@ -275,12 +285,41 @@ public abstract class AddVertexTest extends AbstractGremlinTest {
assertEquals(7, IteratorUtils.count(g.V()));
}
+ @Test
+ @LoadGraphWith(MODERN)
+ @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_PROPERTY)
+ public void g_withSideEffectXa_testX_V_hasLabelXsoftwareX_propertyXtemp_selectXaXX_valueMapXname_tempX() {
+ final Traversal<Vertex, Map<String, List<String>>> traversal = get_g_withSideEffectXa_testX_V_hasLabelXsoftwareX_propertyXtemp_selectXaXX_valueMapXname_tempX();
+ printTraversalForm(traversal);
+ int counter = 0;
+ while (traversal.hasNext()) {
+ counter++;
+ final Map<String, List<String>> valueMap = traversal.next();
+ assertEquals(2, valueMap.size());
+ assertEquals(Collections.singletonList("test"), valueMap.get("temp"));
+ assertTrue(valueMap.get("name").equals(Collections.singletonList("ripple")) || valueMap.get("name").equals(Collections.singletonList("lop")));
+ }
+ assertEquals(2, counter);
+ assertFalse(traversal.hasNext());
+ }
+
+ @Test
+ @LoadGraphWith(MODERN)
+ @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES)
+ @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_PROPERTY)
+ public void g_withSideEffectXa_markoX_addV_propertyXname_selectXaXX_name() {
+ final Traversal<Vertex, String> traversal = get_g_withSideEffectXa_markoX_addV_propertyXname_selectXaXX_name();
+ printTraversalForm(traversal);
+ assertEquals("marko", traversal.next());
+ assertFalse(traversal.hasNext());
+ }
+
public static class Traversals extends AddVertexTest {
@Override
public Traversal<Vertex, Vertex> get_g_VX1X_addVXanimalX_propertyXage_selectXaX_byXageXX_propertyXname_puppyX(final Object v1Id) {
- return g.V(v1Id).as("a").addV("animal").property("age", __.select("a").by("age")).property("name", "puppy");
+ return g.V(v1Id).as("a").addV("animal").property("age", select("a").by("age")).property("name", "puppy");
}
@Override
@@ -332,5 +371,15 @@ public abstract class AddVertexTest extends AbstractGremlinTest {
public Traversal<Vertex, Vertex> get_g_addVXlabel_person_name_stephenX() {
return g.addV(T.label, "person", "name", "stephen");
}
+
+ @Override
+ public Traversal<Vertex, Map<String, List<String>>> get_g_withSideEffectXa_testX_V_hasLabelXsoftwareX_propertyXtemp_selectXaXX_valueMapXname_tempX() {
+ return g.withSideEffect("a", "test").V().hasLabel("software").property("temp", select("a")).valueMap("name", "temp");
+ }
+
+ @Override
+ public Traversal<Vertex, String> get_g_withSideEffectXa_markoX_addV_propertyXname_selectXaXX_name() {
+ return g.withSideEffect("a", "marko").addV().property("name", select("a")).values("name");
+ }
}
}
\ No newline at end of file