Skip to content

Commit 0516a7d

Browse files
committed
TINKERPOP-3195 Prevent SupplyingBarrier Steps inside RepeatStep
SideEfffectCapStep (currently the only SupplyingBarrier) doesn't work inside repeat() and its semantics don't make sense either. It consumes all the starts and can output any traverser which can lead to errors.
1 parent e124db1 commit 0516a7d

File tree

4 files changed

+16
-2
lines changed

4 files changed

+16
-2
lines changed

CHANGELOG.asciidoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ This release also includes changes from <<release-3-7-XXX, 3.7.XXX>>.
7373
* Updated `ElementIdStrategy.getConfiguration()` to help with serialization.
7474
* Fixed issue in `gremlin-go` where `Next()` didn't return the error from the server.
7575
* Changed type for `ReservedKeysVerificationStrategy.keys` in .NET to take a `Set<string>` rather than `List<string>`.
76+
* Changed `StandardVerificationStrategy` to prevent the use of `SupplyingBarriers` inside `repeat()`.
7677
* Fixed bug in `group()` value traversal of the second `by()` where a `CollectingBarrierStep` could produce an unexpected filtering effect when `ReducingBarrierStep` or `SupplyingBarrierStep` instances were not taken into account.
7778
* Changed `DetachedFactory` to special case the handling of `ComputerAdjacentVertex` which doesn't carry properties but still needs to be detachable for OLAP cases.
7879
* Deprecated `ProfilingAware.prepareForProfiling` method preferring to simply `resetBarrierFromValueTraversal` from the `Grouping` interface after strategy application.

docs/src/upgrade/release-3.8.x.asciidoc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,14 @@ version, the `none()` step was used to "throw away" all traversers that passed i
150150
renamed to `discard()`. The `discard()` step with its verb tone arguably makes for a better name for that feature, but
151151
it also helped make room for `none()` to be repurposed as `none(P)` which is a complement to `any(P)` and `all(P) steps.
152152
153+
==== Prevented using cap() inside repeat()
154+
155+
`cap()` inside `repeat()` is now disallowed by the `StandardVerificationStrategy`. Using `cap()` inside `repeat()` would
156+
have led to unexpected results since `cap()` isn't "repeat-aware". Because `cap()` is a `SupplyingBarrier` that reduces
157+
the number of traversers to one, its use inside `repeat()` is limited.
158+
159+
See: link:https://issues.apache.org/jira/browse/TINKERPOP-3195[TINKERPOP-3195]
160+
153161
==== Simplified Comparability Semantics
154162
155163
The previous system of ternary boolean semantics has been replaced with simplified binary semantics. The triggers for

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.SideEffectCapStep;
3030
import org.apache.tinkerpop.gremlin.process.traversal.step.util.ReducingBarrierStep;
3131
import org.apache.tinkerpop.gremlin.process.traversal.step.util.RequirementsStep;
32+
import org.apache.tinkerpop.gremlin.process.traversal.step.util.SupplyingBarrierStep;
3233
import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
3334
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
3435
import org.apache.tinkerpop.gremlin.structure.Graph;
@@ -61,8 +62,10 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
6162
if (Graph.Hidden.isHidden(label))
6263
step.removeLabel(label);
6364
}
64-
if (step instanceof ReducingBarrierStep && step.getTraversal().getParent() instanceof RepeatStep && step.getTraversal().getParent().getGlobalChildren().get(0).getSteps().contains(step))
65-
throw new VerificationException("The parent of a reducing barrier can not be repeat()-step: " + step, traversal);
65+
if ((step instanceof ReducingBarrierStep || step instanceof SupplyingBarrierStep) &&
66+
step.getTraversal().getParent() instanceof RepeatStep &&
67+
step.getTraversal().getParent().getGlobalChildren().get(0).getSteps().contains(step))
68+
throw new VerificationException("The parent of a reducing/supplying barrier can not be repeat()-step: " + step, traversal);
6669

6770
// prevents silly stuff like g.V().emit()
6871
if (step instanceof RepeatStep && null == ((RepeatStep) step).getRepeatTraversal())

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategyTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
import java.util.Arrays;
3535

36+
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.cap;
3637
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out;
3738
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.repeat;
3839
import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.sum;
@@ -58,6 +59,7 @@ public static Iterable<Object[]> data() throws Exception {
5859
{repeat(out().fold().unfold()).times(2), false},
5960
{repeat(sum()).times(2), false},
6061
{repeat(out().count()), false},
62+
{repeat(cap("x")), false},
6163
{__.V().profile(), true},
6264
{__.V().profile("metrics").cap("metrics"), true}
6365
});

0 commit comments

Comments
 (0)