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 2025/06/05 21:40:53 UTC
(tinkerpop) 01/01: wip
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch GValueManager3.8-wip
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit aaedcae8110e3119fddf7a6185a0a295f0bc9d2e
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Thu Jun 5 17:40:27 2025 -0400
wip
---
.../remote/traversal/AbstractRemoteTraversal.java | 5 +
.../gremlin/process/traversal/GValueManager.java | 142 ++++++++++++---
.../gremlin/process/traversal/Traversal.java | 5 +
.../traversal/lambda/AbstractLambdaTraversal.java | 12 +-
.../lambda/CardinalityValueTraversal.java | 7 -
.../process/traversal/step/TraversalParent.java | 2 +
.../strategy/decoration/ConnectiveStrategy.java | 9 +-
.../strategy/optimization/CountStrategy.java | 19 +-
.../strategy/optimization/EarlyLimitStrategy.java | 15 +-
.../optimization/FilterRankingStrategy.java | 3 -
.../process/traversal/util/DefaultTraversal.java | 11 +-
.../process/traversal/util/EmptyTraversal.java | 32 ++--
.../process/traversal/util/TraversalHelper.java | 12 +-
.../gremlin/process/traversal/TraversalTest.java | 3 +
.../traversal/strategy/GValueManagerVerifier.java | 142 +++++++--------
.../decoration/ConnectiveStrategyTest.java | 195 ++++++++++++++-------
.../strategy/optimization/CountStrategyTest.java | 35 +++-
17 files changed, 422 insertions(+), 227 deletions(-)
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/AbstractRemoteTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/AbstractRemoteTraversal.java
index 6f1a7b2969..efd3db418f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/AbstractRemoteTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/remote/traversal/AbstractRemoteTraversal.java
@@ -68,6 +68,11 @@ public abstract class AbstractRemoteTraversal<S,E> implements RemoteTraversal<S,
throw new UnsupportedOperationException("Remote traversals do not support this method");
}
+ @Override
+ public void setGValueManager(final GValueManager gValueManager) {
+ throw new UnsupportedOperationException("Remote traversals do not support this method");
+ }
+
@Override
public List<Step> getSteps() {
throw new UnsupportedOperationException("Remote traversals do not support this method");
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java
index 27e1378e27..14a5fda32b 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java
@@ -19,6 +19,7 @@
package org.apache.tinkerpop.gremlin.process.traversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.stepContract.AddEdgeContract;
import org.apache.tinkerpop.gremlin.process.traversal.step.stepContract.AddVertexContract;
import org.apache.tinkerpop.gremlin.process.traversal.step.stepContract.CallContract;
@@ -34,13 +35,15 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.stepContract.TailCont
import org.apache.tinkerpop.gremlin.structure.Vertex;
import java.io.Serializable;
-import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
/**
* The {@code GValueManager} class is responsible for managing the state of {@link GValue} instances and their
@@ -59,7 +62,7 @@ public final class GValueManager implements Serializable, Cloneable {
this(Collections.EMPTY_MAP, Collections.EMPTY_MAP);
}
- private GValueManager(Map<String, GValue> gValueRegistry, Map<Step, StepContract> stepRegistry) {
+ private GValueManager(final Map<String, GValue> gValueRegistry, final Map<Step, StepContract> stepRegistry) {
this.gValueRegistry.putAll(gValueRegistry);
this.stepRegistry.putAll(stepRegistry);
}
@@ -83,30 +86,14 @@ public final class GValueManager implements Serializable, Cloneable {
* extraction logic for each contract type. If a contract type is found that is not supported, an
* {@link IllegalArgumentException} is thrown.
*/
- public void lock() {
+ public synchronized void lock() {
// can only lock once so if already locked, just return.
if (locked) return;
+ locked = true;
for (StepContract contract : stepRegistry.values()) {
registerGValues(extractGValues(contract));
}
-
- locked = true;
- }
-
- private void registerGValues(Collection<GValue<?>> gValues) {
- for (GValue<?> gValue : gValues) {
- if (gValue.getName() != null) {
- gValueRegistry.put(gValue.getName(), gValue);
- }
- }
- }
-
- private void removeGValues(Collection<GValue<?>> gValues) {
- for (GValue<?> gValue : gValues) {
- gValueRegistry.remove(gValue.getName(), gValue);
- // TODO:: cascade to other steps
- }
}
/**
@@ -116,11 +103,12 @@ public final class GValueManager implements Serializable, Cloneable {
return locked;
}
- public void merge(final GValueManager other) {
+ public void mergeInto(final GValueManager other) {
if (this.locked) throw Traversal.Exceptions.traversalIsLocked();
+
//TODO deal with conflicts
- gValueRegistry.putAll(other.gValueRegistry);
- stepRegistry.putAll(other.stepRegistry);
+ other.gValueRegistry.putAll(gValueRegistry);
+ other.stepRegistry.putAll(stepRegistry);
}
/**
@@ -158,12 +146,69 @@ public final class GValueManager implements Serializable, Cloneable {
/**
* Gets the set of variable names used in this traversal. Note that this set won't be consistent until
- * {@link #lock()} is called first.
+ * {@link #lock()} is called first. Calls to this method prior to locking will force iteration through traversal
+ * steps to real-time gather then variable names.
*/
public Set<String> variableNames() {
- return Collections.unmodifiableSet(gValueRegistry.keySet());
+ if (locked) {
+ return Collections.unmodifiableSet(gValueRegistry.keySet());
+ } else {
+ return Collections.unmodifiableSet(gValues().stream().map(GValue::getName).collect(Collectors.toSet()));
+ }
}
+ /**
+ * Gets the set of {@link GValue} objects used in this traversal. Note that this set won't be consistent until
+ * {@link #lock()} is called first. Calls to this method prior to locking will force iteration through traversal
+ * steps to real-time gather them.
+ */
+ public Set<GValue> gValues() {
+ if (locked) {
+ return Collections.unmodifiableSet(new HashSet<>(gValueRegistry.values()));
+ } else {
+ final Set<GValue> gvalues = new HashSet<>();
+ for (StepContract contract : stepRegistry.values()) {
+ extractGValues(contract).stream().
+ filter(gv -> gv.getName() != null).forEach(gvalues::add);
+ }
+ return Collections.unmodifiableSet(gvalues);
+ }
+ }
+
+ /**
+ * Recursively collect {@link Step} to {@link GValue} mappings from the specified traversal and its children.
+ */
+ public Map<Step, Set<GValue>> gatherStepGValues(final Traversal.Admin<?, ?> traversal) {
+ return gatherStepGValues(traversal, null);
+ }
+
+ private Map<Step, Set<GValue>> gatherStepGValues(final Traversal.Admin<?, ?> traversal, final Map<Step, Set<GValue>> result) {
+ final GValueManager manager = traversal.getGValueManager();
+ final Map<Step, Set<GValue>> r = null == result ? new HashMap<>() : result;
+
+ for (Step step : traversal.getSteps()) {
+ if (manager.isParameterized(step)) {
+ final StepContract contract = manager.getStepContract(step);
+ r.put(step, new HashSet<>(extractGValues(contract)));
+ }
+
+ if (step instanceof TraversalParent) {
+ TraversalParent parent = (TraversalParent) step;
+
+ // Process global children
+ for (Traversal.Admin<?, ?> child : parent.getGlobalChildren()) {
+ gatherStepGValues(child, r);
+ }
+
+ // Process local children
+ for (Traversal.Admin<?, ?> child : parent.getLocalChildren()) {
+ gatherStepGValues(child, r);
+ }
+ }
+ }
+
+ return r;
+ }
/**
* Determines whether the GValueManager contains any registered GValues, steps, or predicates.
@@ -189,13 +234,43 @@ public final class GValueManager implements Serializable, Cloneable {
*/
public void remove(final Step step) {
if (this.locked) throw Traversal.Exceptions.traversalIsLocked();
- StepContract removedContract = stepRegistry.remove(step);
+ final StepContract removedContract = stepRegistry.remove(step);
if (removedContract != null) {
removeGValues(extractGValues(removedContract));
}
}
+ /**
+ * Removes all potential step contracts for a particular traversal. This is not a recursive operation, so use
+ * {@link #removeAllRecursively(Traversal.Admin)} for that case.
+ */
+ public void removeAll(final Traversal.Admin<?, ?> traversal) {
+ if (this.locked) throw Traversal.Exceptions.traversalIsLocked();
+ traversal.getSteps().forEach(this::remove);
+ }
+
+ /**
+ * Removes all potential step contracts for a particular traversal. This is a recursive operation and will examine
+ * all steps in this traversal as well as associated child traversals.
+ */
+ public void removeAllRecursively(final Traversal.Admin<?, ?> traversal) {
+ if (this.locked) throw Traversal.Exceptions.traversalIsLocked();
+
+ for (Step step : traversal.getSteps()) {
+ this.remove(step);
+ if (step instanceof TraversalParent) {
+ TraversalParent parent = (TraversalParent) step;
+ for (Traversal.Admin<?, ?> child : parent.getGlobalChildren()) {
+ removeAllRecursively(child);
+ }
+ for (Traversal.Admin<?, ?> child : parent.getLocalChildren()) {
+ removeAllRecursively(child);
+ }
+ }
+ }
+ }
+
private Collection<GValue<?>> extractPredicateGValues(final P predicate) {
return predicate.getGValueRegistry().getGValues();
}
@@ -388,6 +463,21 @@ public final class GValueManager implements Serializable, Cloneable {
return results;
}
+ private void registerGValues(final Collection<GValue<?>> gValues) {
+ for (GValue<?> gValue : gValues) {
+ if (gValue.getName() != null) {
+ gValueRegistry.put(gValue.getName(), gValue);
+ }
+ }
+ }
+
+ private void removeGValues(final Collection<GValue<?>> gValues) {
+ for (GValue<?> gValue : gValues) {
+ gValueRegistry.remove(gValue.getName(), gValue);
+ // TODO:: cascade to other steps
+ }
+ }
+
@Override
public GValueManager clone() {
return new GValueManager(gValueRegistry, stepRegistry);
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
index 85279d909e..31eb04df4c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
@@ -336,6 +336,11 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, A
*/
public GValueManager getGValueManager();
+ /**
+ * Set the {@link GValueManager} associated with this traversal.
+ */
+ public void setGValueManager(final GValueManager gValueManager);
+
/**
* Add an iterator of {@link Traverser.Admin} objects to the head/start of the traversal. Users should
* typically not need to call this method. For dynamic inject of data, they should use {@link InjectStep}.
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 9a355414ef..ba829235b3 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
@@ -45,7 +45,7 @@ import java.util.Set;
public abstract class AbstractLambdaTraversal<S, E> implements Traversal.Admin<S, E> {
private static final Set<TraverserRequirement> REQUIREMENTS = Collections.singleton(TraverserRequirement.OBJECT);
-
+ protected GValueManager gValueManager = new GValueManager();
protected Traversal.Admin<S, E> bypassTraversal = null;
public void setBypassTraversal(final Traversal.Admin<S, E> bypassTraversal) {
@@ -68,7 +68,15 @@ public abstract class AbstractLambdaTraversal<S, E> implements Traversal.Admin<S
@Override
public GValueManager getGValueManager() {
- return null == this.bypassTraversal ? new GValueManager() : this.bypassTraversal.getGValueManager();
+ return null == this.bypassTraversal ? gValueManager : this.bypassTraversal.getGValueManager();
+ }
+
+ @Override
+ public void setGValueManager(final GValueManager gValueManager) {
+ if (bypassTraversal == null)
+ this.gValueManager = gValueManager;
+ else
+ bypassTraversal.setGValueManager(gValueManager);
}
@Override
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/CardinalityValueTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/CardinalityValueTraversal.java
index 5a65eb7aab..c18e94d036 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/CardinalityValueTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/CardinalityValueTraversal.java
@@ -31,13 +31,11 @@ public final class CardinalityValueTraversal extends AbstractLambdaTraversal {
private final Object value;
private final Bytecode bytecode;
- private final GValueManager gValueManager;
public CardinalityValueTraversal(final VertexProperty.Cardinality cardinality, final Object value) {
this.cardinality = cardinality;
this.value = value;
this.bytecode = new Bytecode(CardinalityValueTraversal.class.getSimpleName(), cardinality.name(), value);
- this.gValueManager = new GValueManager();
}
public static CardinalityValueTraversal from(final Bytecode.Instruction inst) {
@@ -50,11 +48,6 @@ public final class CardinalityValueTraversal extends AbstractLambdaTraversal {
return this.bytecode;
}
- @Override
- public GValueManager getGValueManager() {
- return this.gValueManager;
- }
-
public VertexProperty.Cardinality getCardinality() {
return cardinality;
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java
index 695d2972fc..57f6ddc556 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java
@@ -82,6 +82,8 @@ public interface TraversalParent extends AutoCloseable {
childTraversal.setParent(this);
childTraversal.getSideEffects().mergeInto(this.asStep().getTraversal().getSideEffects());
childTraversal.setSideEffects(this.asStep().getTraversal().getSideEffects());
+ childTraversal.getGValueManager().mergeInto(this.asStep().getTraversal().getGValueManager());
+ childTraversal.setGValueManager(this.asStep().getTraversal().getGValueManager());
return (Traversal.Admin<S, E>) childTraversal;
}
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 d314b4d65c..47fe6670b4 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
@@ -18,6 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration;
+import org.apache.tinkerpop.gremlin.process.traversal.GValueManager;
import org.apache.tinkerpop.gremlin.process.traversal.Step;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
@@ -34,7 +35,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
-import java.util.Collections;
import java.util.List;
import java.util.Set;
@@ -77,7 +77,7 @@ public final class ConnectiveStrategy extends AbstractTraversalStrategy<Traversa
}
private static void processConjunctionMarker(final Class<? extends ConnectiveStep> markerClass, final Traversal.Admin<?, ?> traversal) {
-
+ final GValueManager manager = traversal.getGValueManager();
final List<Step> steps = traversal.getSteps();
for (int i = 0; i < steps.size(); i++) {
final Step step = steps.get(i);
@@ -89,6 +89,7 @@ public final class ConnectiveStrategy extends AbstractTraversalStrategy<Traversa
for (int j = i - 1; j >= 0; i--, j--) {
final Step previousStep = steps.get(j);
if (legalCurrentStep(previousStep)) {
+ // step is moving from one traversal to another. GValue state is unchanged.
connectiveTraversal.addStep(0, previousStep);
traversal.removeStep(previousStep);
} else break;
@@ -99,13 +100,13 @@ public final class ConnectiveStrategy extends AbstractTraversalStrategy<Traversa
while (i < steps.size()) {
final Step nextStep = steps.get(i);
if (legalCurrentStep(nextStep)) {
- if (nextStep.getClass().equals(markerClass) &&
- ((ConnectiveStep) nextStep).getLocalChildren().isEmpty()) {
+ if (nextStep.getClass().equals(markerClass) && ((ConnectiveStep) nextStep).getLocalChildren().isEmpty()) {
final ConnectiveStep<?> nextConnectiveStep = (ConnectiveStep<?>) nextStep;
currentStep.addLocalChild(connectiveTraversal = connectiveTraversal(connectiveTraversal, nextConnectiveStep));
} else {
connectiveTraversal.addStep(nextStep);
}
+ // step is moving from one traversal to another. GValue state is unchanged.
traversal.removeStep(nextStep);
} else break;
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategy.java
index f55de93a14..50bd0fdbdd 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategy.java
@@ -149,9 +149,16 @@ public final class CountStrategy extends AbstractTraversalStrategy<TraversalStra
}
if (highRange != null) {
+ // if this strategy decides it can add a limit() then it means we've accessed the GValue for
+ // logic to reach this point and now intend to insert a new step based on that logic. by doing
+ // this, we nullify the possibility of using that GValue for downstream optimizations as it
+ // would tie the dynamic GValue to a statically constructed literal (i.e. the highRange) that
+ // only applies for this current strategy application.
+ traversal.getGValueManager().remove(isStep);
+
if (useNotStep || dismissCountIs) {
- traversal.asAdmin().removeStep(isStep); // IsStep
- traversal.asAdmin().removeStep(curr); // CountStep
+ TraversalHelper.removeStep(isStep, traversal); // IsStep
+ TraversalHelper.removeStep(curr, traversal); // CountStep
size -= 2;
if (!dismissCountIs) {
if (parent instanceof ConnectiveStep) {
@@ -174,7 +181,7 @@ public final class CountStrategy extends AbstractTraversalStrategy<TraversalStra
inner.addStep(0, prev);
if (pp instanceof EmptyStep || pp instanceof GraphStep ||
!(prev instanceof FilterStep || prev instanceof SideEffectStep)) break;
- traversal.removeStep(prev);
+ TraversalHelper.removeStep(prev, traversal);
prev = pp;
size--;
}
@@ -195,12 +202,6 @@ public final class CountStrategy extends AbstractTraversalStrategy<TraversalStra
}
}
} else {
- // if this strategy decides it can add a limit() then it means we've accessed the GValue for
- // logic to reach this point and now intend to insert a new step based on that logic. by doing
- // this, we nullify the possibility of using that GValue for downstream optimizations as it
- // would tie the dynamic GValue to a statically constructed literal (i.e. the highRange) that
- // only applies for this current strategy application.
- traversal.getGValueManager().remove(isStep);
TraversalHelper.insertBeforeStep(new RangeGlobalStep<>(traversal, 0L, highRange < 0 ? 0 : highRange), curr, traversal);
}
i++;
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/EarlyLimitStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/EarlyLimitStrategy.java
index 1e6e86fe52..9dee35a509 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/EarlyLimitStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/EarlyLimitStrategy.java
@@ -131,9 +131,14 @@ public final class EarlyLimitStrategy
rangeStep = high > low ? new RangeGlobalStep(traversal, low, high) : new NoneStep<>(traversal);
}
remove = merge;
- TraversalHelper.replaceStep(merge ? insertAfter : step, rangeStep, traversal);
+
+ // a step is being permanently removed - remove from manager
+ final Step stepToRemove = merge ? insertAfter : step;
+ traversal.getGValueManager().remove(stepToRemove);
+ TraversalHelper.replaceStep(stepToRemove, rangeStep, traversal);
} else if (!step.getPreviousStep().equals(insertAfter, true)) {
- // move the RangeStep behind the earliest possible map- or sideEffect-step
+ // move the RangeStep behind the earliest possible map- or sideEffect-step.
+ // todo: reconsider clone() b/c it destroys the GValue - any reason to not just move the step?
rangeStep = step.clone();
TraversalHelper.insertAfterStep(rangeStep, insertAfter, traversal);
} else {
@@ -141,7 +146,11 @@ public final class EarlyLimitStrategy
// already the current step's previous step
return step;
}
- if (remove) traversal.removeStep(step);
+ if (remove) {
+ // a step is being permanently removed - remove from manager
+ traversal.getGValueManager().remove(step);
+ traversal.removeStep(step);
+ }
return rangeStep;
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java
index 16c70e89b9..10d7de081f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java
@@ -18,7 +18,6 @@
*/
package org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization;
-import org.apache.tinkerpop.gremlin.process.traversal.GValueManager;
import org.apache.tinkerpop.gremlin.process.traversal.Step;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
@@ -36,11 +35,9 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.filter.OrStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WherePredicateStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WhereTraversalStep;
-import org.apache.tinkerpop.gremlin.process.traversal.step.stepContract.StepContract;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.OrderGlobalStep;
import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
-import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.IdentityRemovalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
import org.javatuples.Pair;
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
index b1294a38c3..29a0e37068 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
@@ -113,6 +113,11 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
return this.gValueManager;
}
+ @Override
+ public void setGValueManager(final GValueManager gValueManager) {
+ this.gValueManager = gValueManager;
+ }
+
@Override
public Traversal.Admin<S, E> asAdmin() {
return this;
@@ -403,11 +408,7 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
if (this.locked) throw Exceptions.traversalIsLocked();
final Step previousStep = this.steps.size() > 0 && index != 0 ? steps.get(index - 1) : null;
final Step nextStep = this.steps.size() > index + 1 ? steps.get(index + 1) : null;
- final Step removedStep = this.steps.remove(index);
-
- // reset state in manager for the removed step
- getGValueManager().remove(removedStep);
-
+ this.steps.remove(index);
if (null != previousStep) previousStep.setNextStep(null == nextStep ? EmptyStep.instance() : nextStep);
if (null != nextStep) nextStep.setPreviousStep(null == previousStep ? EmptyStep.instance() : previousStep);
return (Traversal.Admin<S2, E2>) this;
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/EmptyTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/EmptyTraversal.java
index 389a3da13c..a88f94f9ab 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/EmptyTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/EmptyTraversal.java
@@ -60,9 +60,12 @@ public final class EmptyTraversal<S, E> implements Traversal.Admin<S, E> {
@Override
public GValueManager getGValueManager() {
- return new GValueManager();
+ return null;
}
+ @Override
+ public void setGValueManager(final GValueManager gValueManager) { }
+
@Override
public Traversal.Admin<S, E> asAdmin() {
return this;
@@ -84,19 +87,13 @@ public final class EmptyTraversal<S, E> implements Traversal.Admin<S, E> {
}
@Override
- public void applyStrategies() {
-
- }
+ public void applyStrategies() { }
@Override
- public void addStarts(final Iterator<Traverser.Admin<S>> starts) {
-
- }
+ public void addStarts(final Iterator<Traverser.Admin<S>> starts) { }
@Override
- public void addStart(final Traverser.Admin<S> start) {
-
- }
+ public void addStart(final Traverser.Admin<S> start) { }
@Override
public <E2> Traversal.Admin<S, E2> addStep(final Step<?, E2> step) {
@@ -129,8 +126,7 @@ public final class EmptyTraversal<S, E> implements Traversal.Admin<S, E> {
}
@Override
- public void setSideEffects(final TraversalSideEffects sideEffects) {
- }
+ public void setSideEffects(final TraversalSideEffects sideEffects) { }
@Override
public TraversalStrategies getStrategies() {
@@ -138,9 +134,7 @@ public final class EmptyTraversal<S, E> implements Traversal.Admin<S, E> {
}
@Override
- public void setParent(final TraversalParent step) {
-
- }
+ public void setParent(final TraversalParent step) { }
@Override
public TraversalParent getParent() {
@@ -148,9 +142,7 @@ public final class EmptyTraversal<S, E> implements Traversal.Admin<S, E> {
}
@Override
- public void setStrategies(final TraversalStrategies traversalStrategies) {
-
- }
+ public void setStrategies(final TraversalStrategies traversalStrategies) { }
@Override
public <S2, E2> Traversal.Admin<S2, E2> addStep(final int index, final Step<?, ?> step) throws IllegalStateException {
@@ -183,7 +175,5 @@ public final class EmptyTraversal<S, E> implements Traversal.Admin<S, E> {
}
@Override
- public void setGraph(final Graph graph) {
-
- }
+ public void setGraph(final Graph graph) { }
}
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 c881cca44b..aefada9d8b 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
@@ -23,7 +23,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.GValueManager;
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.TraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.AbstractLambdaTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
@@ -56,7 +55,6 @@ import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.LinkedHashSet;
@@ -185,6 +183,7 @@ public final class TraversalHelper {
*/
public static <S, E> void replaceStep(final Step<S, E> removeStep, final Step<S, E> insertStep, final Traversal.Admin<?, ?> traversal) {
final int i;
+ traversal.getGValueManager().remove(removeStep);
traversal.removeStep(i = stepIndex(removeStep, traversal));
traversal.addStep(i, insertStep);
}
@@ -215,7 +214,7 @@ public final class TraversalHelper {
}
public static <S, E> Step<?, E> insertTraversal(final int insertIndex, final Traversal.Admin<S, E> insertTraversal, final Traversal.Admin<?, ?> traversal) {
- traversal.getGValueManager().merge(insertTraversal.getGValueManager());
+ traversal.getGValueManager().mergeInto(insertTraversal.getGValueManager());
if (0 == traversal.getSteps().size()) {
Step currentStep = EmptyStep.instance();
for (final Step insertStep : insertTraversal.getSteps()) {
@@ -738,9 +737,16 @@ public final class TraversalHelper {
return false;
}
+ public static Traversal.Admin<?, ?> removeStep(final Step<?, ?> stepToRemove, final Traversal.Admin<?, ?> traversal) {
+ traversal.getGValueManager().remove(stepToRemove);
+ return traversal.removeStep(stepToRemove);
+ }
+
public static void removeAllSteps(final Traversal.Admin<?, ?> traversal) {
final int size = traversal.getSteps().size();
for (int i = 0; i < size; i++) {
+ final Step<?, ?> stepToRemove = traversal.getSteps().get(0);
+ traversal.getGValueManager().remove(stepToRemove);
traversal.removeStep(0);
}
}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
index 860c254541..d880af7bd1 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
@@ -280,6 +280,9 @@ public class TraversalTest {
return new GValueManager();
}
+ @Override
+ public void setGValueManager(final GValueManager gValueManager) { }
+
@Override
public List<Step> getSteps() {
return steps;
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java
index 11370391a9..8c23bc94f7 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java
@@ -24,13 +24,13 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.step.GValue;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.stepContract.EdgeLabelContract;
import org.apache.tinkerpop.gremlin.process.traversal.step.stepContract.RangeContract;
import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
+import org.apache.tinkerpop.gremlin.util.CollectionUtil;
-import java.util.Arrays;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -68,11 +68,7 @@ public class GValueManagerVerifier {
}
public BeforeVerifier<S, E> beforeApplying() {
- // Capture pre-strategy state
- final Map<Step, Set<String>> preStepVariables = captureStepVariables(traversal);
- final Set<String> preVariables = new HashSet<>(traversal.getGValueManager().variableNames());
-
- return new BeforeVerifier<>(traversal, preVariables, preStepVariables);
+ return new BeforeVerifier<>(traversal, this);
}
/**
@@ -80,8 +76,10 @@ public class GValueManagerVerifier {
*/
public AfterVerifier<S, E> afterApplying() {
// Capture pre-strategy state
- final Map<Step, Set<String>> preStepVariables = captureStepVariables(traversal);
- final Set<String> preVariables = new HashSet<>(traversal.getGValueManager().variableNames());
+ final GValueManager manager = traversal.getGValueManager();
+ final Map<Step, Set<GValue>> preStepVariables = manager.gatherStepGValues(traversal);
+ final Set<String> preVariables = manager.variableNames();
+ final Set<GValue> preGValues = manager.gValues();
// Apply strategy
final TraversalStrategies strategies = new DefaultTraversalStrategies();
@@ -89,40 +87,31 @@ public class GValueManagerVerifier {
traversal.setStrategies(strategies);
traversal.applyStrategies();
- return new AfterVerifier<>(traversal, preVariables, preStepVariables);
+ return new AfterVerifier<>(traversal, preVariables, preStepVariables, preGValues);
}
+ }
- private Map<Step, Set<String>> captureStepVariables(final Traversal.Admin<?, ?> traversal) {
- final Map<Step, Set<String>> result = new HashMap<>();
- final GValueManager manager = traversal.getGValueManager();
+ /**
+ * Provides verification methods before strategy applications
+ */
+ public static class BeforeVerifier<S, E> extends AbstractVerifier<S, E, BeforeVerifier<S, E>> {
+ private final VerificationBuilder<S, E> verificationBuilder;
- for (Step step : traversal.getSteps()) {
- if (manager.isParameterized(step)) {
- result.put(step, extractStepVariables(step, manager));
- }
- }
- return result;
+ private BeforeVerifier(final Traversal.Admin<S, E> traversal, final VerificationBuilder<S, E> verificationBuilder) {
+ super(traversal);
+ this.verificationBuilder = verificationBuilder;
}
- private Set<String> extractStepVariables(final Step step, final GValueManager manager) {
- final Set<String> variables = new HashSet<>();
- final Object contract = manager.getStepContract(step);
-
- if (contract instanceof EdgeLabelContract) {
- final EdgeLabelContract<GValue<String>> edgeLabelContract = (EdgeLabelContract<GValue<String>>) contract;
- for (GValue<String> label : edgeLabelContract.getEdgeLabels()) {
- if (label.isVariable()) variables.add(label.getName());
- }
- } else if (contract instanceof RangeContract) {
- final RangeContract<GValue<Long>> rangeContract = (RangeContract<GValue<Long>>) contract;
- if (rangeContract.getLowRange().isVariable())
- variables.add(rangeContract.getLowRange().getName());
- if (rangeContract.getHighRange().isVariable())
- variables.add(rangeContract.getHighRange().getName());
- }
- // Add more contract type handling as needed
+ @Override
+ protected BeforeVerifier<S, E> self() {
+ return this;
+ }
- return variables;
+ /**
+ * Applies the strategy and returns the verifier
+ */
+ public AfterVerifier<S, E> afterApplying() {
+ return verificationBuilder.afterApplying();
}
}
@@ -130,10 +119,30 @@ public class GValueManagerVerifier {
* Provides verification methods after strategy application
*/
public static class AfterVerifier<S, E> extends AbstractVerifier<S, E, AfterVerifier<S, E>> {
+ protected final Set<String> preVariables;
+ protected final Set<GValue> preGValues;
+ protected final Map<Step, Set<GValue>> preStepGValues;
+ protected final Map<Step, Set<String>> preStepVariables;
+
private AfterVerifier(final Traversal.Admin<S, E> traversal,
final Set<String> preVariables,
- final Map<Step, Set<String>> preStepVariables) {
- super(traversal, preVariables, preStepVariables);
+ final Map<Step, Set<GValue>> preStepGValues,
+ final Set<GValue> preGValues) {
+ super(traversal);
+ this.preVariables = preVariables;
+ this.preStepGValues = preStepGValues;
+ this.preGValues = preGValues;
+
+ // compute the preStepVariables from the preStepGValues by converting the GValue to their name if
+ // their isVariable is true
+ this.preStepVariables = preStepGValues.entrySet().stream()
+ .collect(java.util.stream.Collectors.toMap(
+ Map.Entry::getKey,
+ e -> e.getValue().stream()
+ .filter(GValue::isVariable)
+ .map(GValue::getName)
+ .collect(java.util.stream.Collectors.toSet())
+ ));
}
@Override
@@ -145,7 +154,7 @@ public class GValueManagerVerifier {
* Verifies that all variables are preserved
*/
public AfterVerifier<S, E> variablesArePreserved() {
- final Set<String> currentVariables = traversal.getGValueManager().variableNames();
+ final Set<String> currentVariables = manager.variableNames();
assertEquals("All variables should be preserved", preVariables, currentVariables);
return this;
}
@@ -154,7 +163,7 @@ public class GValueManagerVerifier {
* Verifies that all variables are removed
*/
public AfterVerifier<S, E> variablesAreRemoved() {
- final Set<String> currentVariables = traversal.getGValueManager().variableNames();
+ final Set<String> currentVariables = manager.variableNames();
assertThat("All variables should be removed", currentVariables.isEmpty(), is(true));
return this;
}
@@ -163,7 +172,7 @@ public class GValueManagerVerifier {
* Verifies that variables have been transferred between steps
*/
public AfterVerifier<S, E> variablesHaveBeenTransferredFrom(final Step originalStep) {
- if (!preStepVariables.containsKey(originalStep)) {
+ if (!preStepGValues.containsKey(originalStep)) {
fail("Original step was not parameterized before strategy application");
}
@@ -182,15 +191,11 @@ public class GValueManagerVerifier {
*/
public static abstract class AbstractVerifier<S, E, T extends AbstractVerifier<S, E, T>> {
protected final Traversal.Admin<S, E> traversal;
- protected final Set<String> preVariables;
- protected final Map<Step, Set<String>> preStepVariables;
+ protected final GValueManager manager;
- protected AbstractVerifier(final Traversal.Admin<S, E> traversal,
- final Set<String> preVariables,
- final Map<Step, Set<String>> preStepVariables) {
+ protected AbstractVerifier(final Traversal.Admin<S, E> traversal) {
this.traversal = traversal;
- this.preVariables = preVariables;
- this.preStepVariables = preStepVariables;
+ this.manager = traversal.getGValueManager();
}
/**
@@ -198,21 +203,11 @@ public class GValueManagerVerifier {
*/
protected abstract T self();
- /**
- * Verifies that specific variables exist
- */
- public T hasVariables(final String... variables) {
- final Set<String> currentVariables = traversal.getGValueManager().variableNames();
- final Set<String> expected = new HashSet<>(Arrays.asList(variables));
- assertEquals("Variables should match expected set", expected, currentVariables);
- return self();
- }
-
/**
* Verifies that GValueManager is empty
*/
public T managerIsEmpty() {
- assertThat("GValueManager should be empty", traversal.getGValueManager().isEmpty(), is(true));
+ assertThat("GValueManager should be empty", manager.isEmpty(), is(true));
return self();
}
@@ -242,7 +237,6 @@ public class GValueManagerVerifier {
public T edgeLabelContractIsValid(final Step step, final int expectedCount,
final String[] expectedNames,
final String[] expectedValues) {
- final GValueManager manager = traversal.getGValueManager();
assertThat("Step should be parameterized", manager.isParameterized(step), is(true));
final EdgeLabelContract<GValue<String>> contract = manager.getStepContract(step);
@@ -265,7 +259,6 @@ public class GValueManagerVerifier {
*/
public T rangeContractIsValid(final Step step, final long expectedLow, final long expectedHigh,
final String lowName, final String highName) {
- final GValueManager manager = traversal.getGValueManager();
assertThat("Step should be parameterized", manager.isParameterized(step), is(true));
final RangeContract<GValue<Long>> contract = manager.getStepContract(step);
@@ -278,21 +271,22 @@ public class GValueManagerVerifier {
return self();
}
- }
- /**
- * Provides verification methods before strategy applications
- */
- public static class BeforeVerifier<S, E> extends AbstractVerifier<S, E, BeforeVerifier<S, E>> {
- private BeforeVerifier(final Traversal.Admin<S, E> traversal,
- final Set<String> preVariables,
- final Map<Step, Set<String>> preStepVariables) {
- super(traversal, preVariables, preStepVariables);
+ /**
+ * Verifies that specific variables exist
+ */
+ public T hasVariables(final String... variables) {
+ return hasVariables(CollectionUtil.asSet(variables));
}
- @Override
- protected BeforeVerifier<S, E> self() {
- return this;
+ /**
+ * Verifies that specific variables exist
+ */
+ public T hasVariables(final Set<String> variables) {
+ // Get variables from the current traversal and all child traversals
+ final Set<String> currentVariables = manager.variableNames();
+ assertEquals("Variables should match expected set", variables, currentVariables);
+ return self();
}
}
}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategyTest.java
index cffac716d5..7ebe1e46c2 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategyTest.java
@@ -18,97 +18,160 @@
*/
package org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration;
+import org.apache.tinkerpop.gremlin.process.traversal.GValueManager;
import org.apache.tinkerpop.gremlin.process.traversal.Translator;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
+import org.apache.tinkerpop.gremlin.process.traversal.step.GValue;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.IsStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.RangeGlobalStep;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.GValueManagerVerifier;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.CountStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.translator.GroovyTranslator;
import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies;
+import org.apache.tinkerpop.gremlin.util.CollectionUtil;
import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import java.util.Arrays;
+import java.util.Set;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.eq;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.gt;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.gte;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.inside;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.lt;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.lte;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.neq;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.outside;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.within;
+import static org.apache.tinkerpop.gremlin.process.traversal.P.without;
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has;
import static org.junit.Assert.assertEquals;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
-@RunWith(Parameterized.class)
+@RunWith(Enclosed.class)
public class ConnectiveStrategyTest {
- private static final Translator.ScriptTranslator translator = GroovyTranslator.of("__");
+ @RunWith(Parameterized.class)
+ public static class StandardTest {
- @Parameterized.Parameter(value = 0)
- public Traversal.Admin original;
+ private static final Translator.ScriptTranslator translator = GroovyTranslator.of("__");
- @Parameterized.Parameter(value = 1)
- public Traversal optimized;
+ @Parameterized.Parameter(value = 0)
+ public Traversal.Admin original;
- void applyConnectiveStrategy(final Traversal traversal) {
- final TraversalStrategies strategies = new DefaultTraversalStrategies();
- strategies.addStrategies(ConnectiveStrategy.instance());
- traversal.asAdmin().setStrategies(strategies);
- traversal.asAdmin().applyStrategies();
- }
+ @Parameterized.Parameter(value = 1)
+ public Traversal optimized;
+
+ void applyConnectiveStrategy(final Traversal traversal) {
+ final TraversalStrategies strategies = new DefaultTraversalStrategies();
+ strategies.addStrategies(ConnectiveStrategy.instance());
+ traversal.asAdmin().setStrategies(strategies);
+ traversal.asAdmin().applyStrategies();
+ }
- @Test
- public void doTest() {
- final String repr = translator.translate(original.getBytecode()).getScript();
- applyConnectiveStrategy(original);
- assertEquals(repr, optimized, original);
+ @Test
+ public void doTest() {
+ final String repr = translator.translate(original.getBytecode()).getScript();
+ applyConnectiveStrategy(original);
+ assertEquals(repr, optimized, original);
+ }
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Iterable<Object[]> generateTestParameters() {
+ return Arrays.asList(new Traversal[][]{
+ {__.has("name", "marko").and().has("name", "marko").and().has("name", "marko"), __.and(has("name", "marko"), __.has("name", "marko"), __.has("name", "marko"))},
+ {__.has("name", "stephen").or().where(__.out("knows").has("name", "stephen")), __.or(__.has("name", "stephen"), __.where(__.out("knows").has("name", "stephen")))},
+ {__.out("a").out("b").and().out("c").or().out("d"), __.or(__.and(__.out("a").out("b"), __.out("c")), __.out("d"))},
+ {__.as("1").out("a").out("b").as("2").and().as("3").out("c").as("4").or().as("5").out("d").as("6"), __.or(__.and(__.as("1").out("a").out("b").as("2"), __.as("3").out("c").as("4")), __.as("5").out("d").as("6"))},
+ {__.as("1").out("a").out("b").and().as("3").out("c").or().as("5").out("d"), __.or(__.and(__.as("1").out("a").out("b"), __.as("3").out("c")), __.as("5").out("d"))},
+ {__.as("1").out("a").out("b").or().as("3").out("c").and().as("5").out("d"), __.or(__.as("1").out("a").out("b"), __.and(__.as("3").out("c"), __.as("5").out("d")))},
+ {__.as("a").out().as("b").and().as("c").in().as("d"), __.and(__.as("a").out().as("b"), __.as("c").in().as("d"))},
+ {__.union(__.as("a").out("l1").as("b").or().as("c").in("l2").as("d"), __.as("e").out("l3").as("f").and().as("g").in("l4").as("h")), __.union(__.or(__.as("a").out("l1").as("b"), __.as("c").in("l2").as("d")), __.and(__.as("e").out("l3").as("f"), __.as("g").in("l4").as("h")))},
+ {__
+ .as("a1").out("a").as("a2").or()
+ .as("b1").out("b").as("b2").and()
+ .as("c1").out("c").as("c2").or()
+ .as("d1").out("d").as("d2").or()
+ .as("e1").out("e").as("e2").and()
+ .as("f1").out("f").as("f2").and()
+ .as("g1").out("g").as("g2").or()
+ .as("h1").out("h").as("h2").or(__
+ .as("i1").out("i").as("i2").or()
+ .as("j1").out("j").as("j2").and()
+ .as("k1").out("k").as("k2")).and()
+ .as("l1").out("l").as("l2").and()
+ .as("m1").out("m").as("m2").and()
+ .as("n1").out("n").as("n2"),
+ // EXPECT:
+ __.or(
+ __.as("a1").out("a").as("a2"),
+ __.and(
+ __.as("b1").out("b").as("b2"),
+ __.as("c1").out("c").as("c2")),
+ __.as("d1").out("d").as("d2"),
+ __.and(
+ __.as("e1").out("e").as("e2"),
+ __.as("f1").out("f").as("f2"),
+ __.as("g1").out("g").as("g2")),
+ __.and(
+ __.as("h1").out("h").as("h2").or(
+ __.or(
+ __.as("i1").out("i").as("i2"),
+ __.and(
+ __.as("j1").out("j").as("j2"),
+ __.as("k1").out("k").as("k2")))),
+ __.as("l1").out("l").as("l2"),
+ __.as("m1").out("m").as("m2"),
+ __.as("n1").out("n").as("n2")))
+ }
+ });
+ }
}
- @Parameterized.Parameters(name = "{0}")
- public static Iterable<Object[]> generateTestParameters() {
- return Arrays.asList(new Traversal[][]{
- {__.has("name", "marko").and().has("name", "marko").and().has("name", "marko"), __.and(has("name", "marko"), __.has("name", "marko"), __.has("name", "marko"))},
- {__.has("name", "stephen").or().where(__.out("knows").has("name", "stephen")), __.or(__.has("name", "stephen"), __.where(__.out("knows").has("name", "stephen")))},
- {__.out("a").out("b").and().out("c").or().out("d"), __.or(__.and(__.out("a").out("b"), __.out("c")), __.out("d"))},
- {__.as("1").out("a").out("b").as("2").and().as("3").out("c").as("4").or().as("5").out("d").as("6"), __.or(__.and(__.as("1").out("a").out("b").as("2"), __.as("3").out("c").as("4")), __.as("5").out("d").as("6"))},
- {__.as("1").out("a").out("b").and().as("3").out("c").or().as("5").out("d"), __.or(__.and(__.as("1").out("a").out("b"), __.as("3").out("c")), __.as("5").out("d"))},
- {__.as("1").out("a").out("b").or().as("3").out("c").and().as("5").out("d"), __.or(__.as("1").out("a").out("b"), __.and(__.as("3").out("c"), __.as("5").out("d")))},
- {__.as("a").out().as("b").and().as("c").in().as("d"), __.and(__.as("a").out().as("b"), __.as("c").in().as("d"))},
- {__.union(__.as("a").out("l1").as("b").or().as("c").in("l2").as("d"), __.as("e").out("l3").as("f").and().as("g").in("l4").as("h")), __.union(__.or(__.as("a").out("l1").as("b"), __.as("c").in("l2").as("d")), __.and(__.as("e").out("l3").as("f"), __.as("g").in("l4").as("h")))},
- {__
- .as("a1").out("a").as("a2").or()
- .as("b1").out("b").as("b2").and()
- .as("c1").out("c").as("c2").or()
- .as("d1").out("d").as("d2").or()
- .as("e1").out("e").as("e2").and()
- .as("f1").out("f").as("f2").and()
- .as("g1").out("g").as("g2").or()
- .as("h1").out("h").as("h2").or(__
- .as("i1").out("i").as("i2").or()
- .as("j1").out("j").as("j2").and()
- .as("k1").out("k").as("k2")).and()
- .as("l1").out("l").as("l2").and()
- .as("m1").out("m").as("m2").and()
- .as("n1").out("n").as("n2"),
- // EXPECT:
- __.or(
- __.as("a1").out("a").as("a2"),
- __.and(
- __.as("b1").out("b").as("b2"),
- __.as("c1").out("c").as("c2")),
- __.as("d1").out("d").as("d2"),
- __.and(
- __.as("e1").out("e").as("e2"),
- __.as("f1").out("f").as("f2"),
- __.as("g1").out("g").as("g2")),
- __.and(
- __.as("h1").out("h").as("h2").or(
- __.or(
- __.as("i1").out("i").as("i2"),
- __.and(
- __.as("j1").out("j").as("j2"),
- __.as("k1").out("k").as("k2")))),
- __.as("l1").out("l").as("l2"),
- __.as("m1").out("m").as("m2"),
- __.as("n1").out("n").as("n2")))
- }
- });
+ /**
+ * Tests that {@link GValueManager} is properly maintaining state in cases where traversals containing a
+ * {@link GValue} move within a new parent traversal.
+ */
+ @RunWith(Parameterized.class)
+ public static class GValueMaintainedAfterMovingWithinParentTest {
+
+ @Parameterized.Parameter(value = 0)
+ public Traversal.Admin<?, ?> traversal;
+
+ @Parameterized.Parameter(value = 1)
+ public Set<String> expectedVariables;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Iterable<Object[]> generateTestParameters() {
+ return Arrays.asList(new Object[][]{
+ {__.has("name", GValue.ofString("x", "marko")).and().has("name", GValue.ofString("x", "marko")).asAdmin(), CollectionUtil.asSet("x")},
+ {__.has("name", GValue.ofString("x", "marko")).and().has("name", GValue.ofString("x", "marko")).and().has("name", GValue.ofString("x", "marko")).asAdmin(), CollectionUtil.asSet("x")},
+ {__.has("name", GValue.ofString("x", "marko")).and().has("name", GValue.ofString("y", "josh")).asAdmin(), CollectionUtil.asSet("x", "y")},
+ {__.has("name", GValue.ofString("x", "marko")).and().has("name", GValue.ofString("y", "josh")).and().has("name", GValue.ofString("z", "vadas")).asAdmin(), CollectionUtil.asSet("x", "y", "z")},
+ {__.has("name", GValue.ofString("x", "stephen")).or().where(__.out(GValue.ofString("y", "knows")).has("name", GValue.ofString("x", "stephen"))), CollectionUtil.asSet("x", "y")},
+ {__.out(GValue.ofString("w", "a")).out(GValue.ofString("x", "b")).and().out(GValue.ofString("y", "c")).or().out(GValue.ofString("z", "d")), CollectionUtil.asSet("w", "x", "y", "z")},
+ {__.as("a").out(GValue.ofString("x", "knows")).as("b").and().as("c").in(GValue.ofString("y", "likes")).as("d"), CollectionUtil.asSet("x", "y")},
+ {__.as("1").out(GValue.ofString("w", "a")).out(GValue.ofString("x", "b")).and().as("3").out(GValue.ofString("y", "c")).or().as("5").out(GValue.ofString("z", "d")), CollectionUtil.asSet("w", "x", "y", "z")},
+ {__.as("1").out(GValue.ofString("w", "a")).out(GValue.ofString("x", "b")).or().as("3").out(GValue.ofString("y", "c")).and().as("5").out(GValue.ofString("z", "d")), CollectionUtil.asSet("w", "x", "y", "z")},
+ });
+ }
+
+ @Test
+ public void shouldMaintainGValueManagerState() {
+ // need to be sure that all variables that were added did not get lost in traversal rewriting
+ // where steps are being moved from the top level traversal down to child traversals.
+ GValueManagerVerifier.verify(traversal, ConnectiveStrategy.instance()).
+ beforeApplying().
+ hasVariables(expectedVariables).
+ afterApplying().
+ hasVariables(expectedVariables);
+ }
}
}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategyTest.java
index 163a390e1d..bcb816d7f6 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategyTest.java
@@ -30,6 +30,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.strategy.GValueManagerVeri
import org.apache.tinkerpop.gremlin.process.traversal.translator.GroovyTranslator;
import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
+import org.apache.tinkerpop.gremlin.util.CollectionUtil;
import org.junit.Test;
import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
@@ -37,6 +38,7 @@ import org.junit.runners.Parameterized;
import java.util.Arrays;
import java.util.List;
+import java.util.Set;
import static org.apache.tinkerpop.gremlin.process.traversal.P.eq;
import static org.apache.tinkerpop.gremlin.process.traversal.P.gt;
@@ -227,7 +229,6 @@ public class CountStrategyTest {
{__.out().count().is(without(GValue.ofInteger("x", 2), GValue.ofInteger("y", 6), GValue.ofInteger("z", 4)))},
{__.map(__.count().is(GValue.of("x", 0)))},
{__.flatMap(__.count().is(GValue.of("x", 0)))},
- {__.flatMap(__.count().is(GValue.of("x", 0))).as("a")},
{__.filter(__.count().is(GValue.of("x", 0))).as("a")},
{__.filter(__.count().is(GValue.of("x", 0)))},
{__.sideEffect(__.count().is(GValue.of("x", 0)))},
@@ -235,7 +236,7 @@ public class CountStrategyTest {
{__.count().is(GValue.of("x", 0)).store("x")},
{__.repeat(__.out()).until(__.outE().count().is(GValue.of("x", 0)))},
{__.repeat(__.out()).until(__.out().out().values("name").count().is(GValue.of("x", 0)))},
- {__.repeat(__.out()).until(__.out().out().properties("age").has("x").count().is(GValue.of("x", 0)))},
+ {__.repeat(__.out()).until(__.out().out().properties("age").has("x").count().is(GValue.of("x", 0)))}, /// //////////
{__.repeat(__.out()).emit(__.outE().count().is(GValue.of("x", 0)))},
{__.where(__.outE().hasLabel("created").count().is(GValue.of("x", 0)))},
{__.where(__.out().outE().hasLabel("created").count().is(GValue.of("x", 0)))},
@@ -290,14 +291,40 @@ public class CountStrategyTest {
@Test
public void shouldMaintainGValueManagerState() {
+ // After applying strategies, all IsSteps should have no GValue references
GValueManagerVerifier.verify(traversal, CountStrategy.instance()).
beforeApplying().
- stepsOfClassAreParameterized(true, IsStep.class);
+ stepsOfClassAreParameterized(true, IsStep.class).
+ afterApplying().
+ managerIsEmpty();
+ }
+ }
+
+ /**
+ * Tests that {@link GValueManager} is properly maintaining state in cases strategy logic does not depend on
+ * the content of the {@link GValue}.
+ */
+ @RunWith(Parameterized.class)
+ public static class GValuePreservedSinceStrategyWasNotInformedByGValueTest {
+
+ @Parameterized.Parameter(value = 0)
+ public Traversal.Admin<?, ?> traversal;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Iterable<Object[]> generateTestParameters() {
+ return Arrays.asList(new Object[][]{
+ {__.flatMap(__.count().is(GValue.of("x", 0))).as("a")},
+ });
+ }
+ @Test
+ public void shouldMaintainGValueManagerState() {
// After applying strategies, all IsSteps should have no GValue references
GValueManagerVerifier.verify(traversal, CountStrategy.instance()).
+ beforeApplying().
+ stepsOfClassAreParameterized(true, IsStep.class).
afterApplying().
- managerIsEmpty();
+ variablesArePreserved();
}
}
}
\ No newline at end of file