Skip to content

Commit

Permalink
Delay discard until 4.0.0
Browse files Browse the repository at this point in the history
  • Loading branch information
ryn5 committed Dec 15, 2023
1 parent 8036ea2 commit 6490f4a
Show file tree
Hide file tree
Showing 26 changed files with 49 additions and 209 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-3-7-2]]
=== TinkerPop 3.7.2 (NOT OFFICIALLY RELEASED YET)
* Deprecated `none()` in favor of `discard()` to make room for a list filtering `none()` step.
* Deprecated `none()`, which will be renamed to `discard()` in release 4.0.0.
[[release-3-7-1]]
Expand Down
20 changes: 2 additions & 18 deletions docs/src/reference/the-traversal.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1448,21 +1448,6 @@ g.V().values("name").fold().difference(__.V().limit(2).values("name").fold())
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#difference(java.lang.Object)++[`difference(Object)`]
link:++https://tinkerpop.apache.org/docs/x.y.z/dev/provider/#difference-step++[`Semantics`]
[[discard-step]]
=== Discard Step
The `discard()`-step (*filter*) filters all objects from a traversal stream. It is especially useful for traversals
that are executed remotely where returning results is not useful and the traversal is only meant to generate
side-effects. Choosing not to return results saves in serialization and network costs as the objects are filtered on
the remote end and not returned to the client side. Typically, this step does not need to be used directly and is
quietly used by the `iterate()` terminal step which appends `discard()` to the traversal before actually cycling through
results.
*Additional References*
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/Traversal.html#discard()++[`discard()`]
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/Traversal.html#iterate()++[`iterate()`]
[[disjunct-step]]
=== Disjunct Step
Expand Down Expand Up @@ -3170,12 +3155,11 @@ the remote end and not returned to the client side. Typically, this step does no
quietly used by the `iterate()` terminal step which appends `none()` to the traversal before actually cycling through
results.
NOTE: As of release 3.7.2, `none()` has been deprecated in favor of `discard()` and is no longer used by `iterate()`.
NOTE: As of release 3.7.2, `none()` has been deprecated and will be renamed to `discard()` in 4.0.0.
*Additional References*
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/Traversal.html#none()++[`discard()`]
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/Traversal.html#discard()++[`discard()`]
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/Traversal.html#none()++[`none()`]
link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/Traversal.html#iterate()++[`iterate()`]
[[not-step]]
Expand Down
4 changes: 2 additions & 2 deletions docs/src/upgrade/release-3.7.x.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ complete list of all the modifications that are part of this release.
==== Deprecated `none()` Step
`none()`, which is primarily used by `iterate()` to discard traversal results in remote contexts, has been deprecated
in favor of `discard()`. Going forward, users should instead use `discard()` as `none()` will be removed in future versions.
and will be renamed to `discard()` in release 4.0.0.
=== Upgrading for Providers
==== Renaming None Step to Discard
NoneStep, which is primarily used by `iterate()` to discard traversal results in remote contexts, has been deprecated
in favor of DiscardStep and will be removed in the next major release to make room for a list filtering step NoneStep.
and will be renamed to DiscardStep in release 4.0.0 to make room for a list filtering NoneStep.
== TinkerPop 3.7.1
*Release Date: November 20, 2023*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1247,10 +1247,6 @@ protected void notImplemented(final ParseTree ctx) {
* {@inheritDoc}
*/
@Override public T visitTraversalTerminalMethod_toBulkSet(final GremlinParser.TraversalTerminalMethod_toBulkSetContext ctx) { notImplemented(ctx); return null; }
/**
* {@inheritDoc}
*/
@Override public T visitTraversalSelfMethod_discard(final GremlinParser.TraversalSelfMethod_discardContext ctx) { notImplemented(ctx); return null; }
/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,6 @@ public Traversal visitTraversalSelfMethod(final GremlinParser.TraversalSelfMetho
return visitChildren(ctx);
}

/**
* {@inheritDoc}
*/
@Override
public Traversal visitTraversalSelfMethod_discard(final GremlinParser.TraversalSelfMethod_discardContext ctx) {
this.traversal = traversal.discard();
return this.traversal;
}

/**
* {@inheritDoc}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.apache.tinkerpop.gremlin.process.remote.traversal.step.map.RemoteStep;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DiscardStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NoneStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.InjectStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.ProfileSideEffectStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.SideEffectCapStep;
Expand Down Expand Up @@ -75,7 +75,6 @@ private Symbols() {
}

public static final String profile = "profile";
public static final String discard = "none";
public static final String none = "none";
}

Expand Down Expand Up @@ -202,7 +201,7 @@ public default <C extends Collection<E>> C fill(final C collection) {
public default <A, B> Traversal<A, B> iterate() {
try {
if (!this.asAdmin().isLocked()) {
this.discard();
this.none();
this.asAdmin().applyStrategies();
}
// use the end step so the results are bulked
Expand All @@ -222,26 +221,14 @@ public default <A, B> Traversal<A, B> iterate() {
* signal to remote servers that {@link #iterate()} was called. While it may be directly used, it is often a sign
* that a traversal should be re-written in another form.
*
* @return the updated traversal with respective {@link DiscardStep}.
*/
public default Traversal<S, E> discard() {
this.asAdmin().getBytecode().addStep(Symbols.discard);
return this.asAdmin().addStep(new DiscardStep<>(this.asAdmin()));
}

/**
* Filter all traversers in the traversal. This step has narrow use cases and is primarily intended for use as a
* signal to remote servers that {@link #iterate()} was called. While it may be directly used, it is often a sign
* that a traversal should be re-written in another form.
*
* @return the updated traversal with respective {@link DiscardStep}.
* @return the updated traversal with respective {@link NoneStep}.
*
* @deprecated As of release 3.7.2, replaced by {@link #discard()}.
* @deprecated As of release 3.7.2 and will be renamed to discard() in 4.0.0.
*/
@Deprecated
public default Traversal<S, E> none() {
this.asAdmin().getBytecode().addStep(Symbols.none);
return this.asAdmin().addStep(new DiscardStep<>(this.asAdmin()));
return this.asAdmin().addStep(new NoneStep<>(this.asAdmin()));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.CoinStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.ConnectiveStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DedupGlobalStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DiscardStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NoneStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DropStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.IsStep;
Expand Down Expand Up @@ -2006,21 +2006,9 @@ public default GraphTraversal<S, E> filter(final Traversal<?, ?> filterTraversal
* signal to remote servers that {@link #iterate()} was called. While it may be directly used, it is often a sign
* that a traversal should be re-written in another form.
*
* @return the updated traversal with respective {@link DiscardStep}.
*/
@Override
default GraphTraversal<S, E> discard() {
return (GraphTraversal<S, E>) Traversal.super.discard();
}

/**
* Filter all traversers in the traversal. This step has narrow use cases and is primarily intended for use as a
* signal to remote servers that {@link #iterate()} was called. While it may be directly used, it is often a sign
* that a traversal should be re-written in another form.
*
* @return the updated traversal with respective {@link DiscardStep}.
* @return the updated traversal with respective {@link NoneStep}.
*
* @deprecated As of release 3.7.2, replaced by {@link #discard()}.
* @deprecated As of release 3.7.2 and will be renamed to discard() in 4.0.0.
*/
@Deprecated
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -805,13 +805,6 @@ public static <A> GraphTraversal<A, Set<?>> difference(final Object values) {
return __.<A>start().difference(values);
}

/**
* @see GraphTraversal#discard()
*/
public static <A> GraphTraversal<A, A> discard() {
return __.<A>start().discard();
}

/**
* @see GraphTraversal#disjunct(Object)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
public final class DiscardStep<S> extends FilterStep<S> {
public final class NoneStep<S> extends FilterStep<S> {

public DiscardStep(final Traversal.Admin traversal) {
public NoneStep(final Traversal.Admin traversal) {
super(traversal);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.step.SideEffectCapable;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DiscardStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NoneStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.RangeGlobalStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.MapStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.ProfileSideEffectStep;
Expand All @@ -38,15 +38,15 @@
* This strategy looks for {@link RangeGlobalStep}s that can be moved further left in the traversal and thus be applied
* earlier. It will also try to merge multiple {@link RangeGlobalStep}s into one.
* If the logical consequence of one or multiple {@link RangeGlobalStep}s is an empty result, the strategy will remove
* as many steps as possible and add a {@link DiscardStep} instead.
* as many steps as possible and add a {@link NoneStep} instead.
*
* @author Daniel Kuppitz (http://gremlin.guru)
* @example <pre>
* __.out().valueMap().limit(5) // becomes __.out().limit(5).valueMap()
* __.outE().range(2, 10).valueMap().limit(5) // becomes __.outE().range(2, 7).valueMap()
* __.outE().limit(5).valueMap().range(2, -1) // becomes __.outE().range(2, 5).valueMap()
* __.outE().limit(5).valueMap().range(5, 10) // becomes __.outE().discard()
* __.outE().limit(5).valueMap().range(5, 10).cap("a") // becomes __.outE().discard().cap("a")
* __.outE().limit(5).valueMap().range(5, 10) // becomes __.outE().none()
* __.outE().limit(5).valueMap().range(5, 10).cap("a") // becomes __.outE().none().cap("a")
* </pre>
*/
public final class EarlyLimitStrategy
Expand All @@ -73,10 +73,10 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
// previous RangeStep; keep the RangeStep's labels at its preceding step
TraversalHelper.copyLabels(step, step.getPreviousStep(), true);
insertAfter = moveRangeStep((RangeGlobalStep) step, insertAfter, traversal, merge);
if (insertAfter instanceof DiscardStep) {
// any step besides a SideEffectCapStep after a DiscardStep would be pointless
final int discardStepIndex = TraversalHelper.stepIndex(insertAfter, traversal);
for (i = j - 2; i > discardStepIndex; i--) {
if (insertAfter instanceof NoneStep) {
// any step besides a SideEffectCapStep after a NoneStep would be pointless
final int noneStepIndex = TraversalHelper.stepIndex(insertAfter, traversal);
for (i = j - 2; i > noneStepIndex; i--) {
if (!(steps.get(i) instanceof SideEffectCapStep) && !(steps.get(i) instanceof ProfileSideEffectStep)) {
traversal.removeStep(i);
}
Expand Down Expand Up @@ -106,7 +106,7 @@ private Step moveRangeStep(final RangeGlobalStep step, final Step insertAfter, f
if (insertAfter instanceof RangeGlobalStep) {
// there's a previous RangeStep which might affect the effective range of the current RangeStep
// recompute this step's low and high; if the result is still a valid range, create a new RangeStep,
// otherwise a DiscardStep
// otherwise a NoneStep
final RangeGlobalStep other = (RangeGlobalStep) insertAfter;
final long low = other.getLowRange() + step.getLowRange();
if (other.getHighRange() == -1L) {
Expand All @@ -116,11 +116,11 @@ private Step moveRangeStep(final RangeGlobalStep step, final Step insertAfter, f
if (low < high) {
rangeStep = new RangeGlobalStep(traversal, low, high);
} else {
rangeStep = new DiscardStep<>(traversal);
rangeStep = new NoneStep<>(traversal);
}
} else {
final long high = Math.min(other.getLowRange() + step.getHighRange(), other.getHighRange());
rangeStep = high > low ? new RangeGlobalStep(traversal, low, high) : new DiscardStep<>(traversal);
rangeStep = high > low ? new RangeGlobalStep(traversal, low, high) : new NoneStep<>(traversal);
}
remove = merge;
TraversalHelper.replaceStep(merge ? insertAfter : step, rangeStep, traversal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.step.Barrier;
import org.apache.tinkerpop.gremlin.process.traversal.step.PathProcessor;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DiscardStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NoneStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DropStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.ElementStep;
Expand Down Expand Up @@ -113,14 +113,14 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
(i > 0 || ((GraphStep) step).getIds().length >= BIG_START_SIZE ||
(((GraphStep) step).getIds().length == 0 && !(step.getNextStep() instanceof HasStep))))) {

// DiscardStep, EmptyStep signify the end of the traversal where no barriers are really going to be
// NoneStep, EmptyStep signify the end of the traversal where no barriers are really going to be
// helpful after that. ProfileSideEffectStep means the traversal had profile() called on it and if
// we don't account for that a barrier will inject at the end of the traversal where it wouldn't
// be otherwise. LazyBarrierStrategy executes before the finalization strategy of ProfileStrategy
// so additionally injected ProfileSideEffectStep instances should not have effect here.
if (foundFlatMap && !labeledPath &&
!(step.getNextStep() instanceof Barrier) &&
!(step.getNextStep() instanceof DiscardStep) &&
!(step.getNextStep() instanceof NoneStep) &&
!(step.getNextStep() instanceof EmptyStep) &&
!(step.getNextStep() instanceof ProfileSideEffectStep)) {
final Step noOpBarrierStep = new NoOpBarrierStep<>(traversal, MAX_BARRIER_SIZE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import org.apache.tinkerpop.gremlin.process.traversal.step.PathProcessor;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.branch.RepeatStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DiscardStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NoneStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.NoOpBarrierStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
Expand Down Expand Up @@ -136,7 +136,7 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
!(currentStep instanceof Barrier) &&
!(currentStep.getNextStep() instanceof Barrier) &&
!(currentStep.getTraversal().getParent() instanceof MatchStep) &&
!(currentStep.getNextStep() instanceof DiscardStep) &&
!(currentStep.getNextStep() instanceof NoneStep) &&
!(currentStep.getNextStep() instanceof EmptyStep))
TraversalHelper.insertAfterStep(new NoOpBarrierStep<>(traversal, this.standardBarrierSize), currentStep, traversal);
}
Expand Down
Loading

0 comments on commit 6490f4a

Please sign in to comment.