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:52 UTC
(tinkerpop) branch GValueManager3.8-wip created (now aaedcae811)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch GValueManager3.8-wip
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
at aaedcae811 wip
This branch includes the following new commits:
new aaedcae811 wip
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(tinkerpop) 01/01: wip
Posted by sp...@apache.org.
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