[
https://issues.apache.org/jira/browse/TINKERPOP-3202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18032539#comment-18032539
]
ASF GitHub Bot commented on TINKERPOP-3202:
-------------------------------------------
Cole-Greer commented on code in PR #3251:
URL: https://github.com/apache/tinkerpop/pull/3251#discussion_r2456032981
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/RangeGlobalStep.java:
##########
@@ -157,10 +163,26 @@ public void processAllStarts() {
}
+ private AtomicLong getCounter(final Traverser.Admin<S> traverser) {
+ if (isInsideLoop()) {
+ final String counterKey = getCounterKey(traverser);
+ return loopCounters.computeIfAbsent(counterKey, k -> new
AtomicLong(0L));
+ } else {
+ return this.singleCounter;
+ }
+ }
+
+ private boolean isInsideLoop() {
Review Comment:
There is some risk that if this method is called before a traversal is
finalized (for example before "this" traversal is added to a new parent), it
may store an incorrect state in `this.insideLoop` which appears unrecoverable.
It looks to be handled properly here, but could benefit from a comment warning
future devs of such considerations.
> Limit and range semantics change when used inside repeat
> --------------------------------------------------------
>
> Key: TINKERPOP-3202
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3202
> Project: TinkerPop
> Issue Type: Improvement
> Components: process
> Affects Versions: 3.7.4
> Reporter: Andrea Child
> Priority: Major
> Labels: breaking
>
> Original dev list post:
> [https://lists.apache.org/thread/nclqpdb5d9wm6cgdxkk39k0co7gt564l]
> Currently, limit() and range() steps inside repeat() maintain global
> counters that persist across repeat iterations, leading to strange behaviour.
> To demonstrate, consider the following limit() traversals for the Grateful
> Dead toy graph which are querying for pairs of song sequences starting with
> the song 'JAM'.
> This repeat() traversal does not produce results because the limit(2)
> counter reaches the global limit after the first iteration (note that
> RepeatUnrollStrategy is disabled so that the repeat() step is not stripped
> by strategy optimization):
> gremlin>
> g.withoutStrategies(RepeatUnrollStrategy).V().has('name','JAM').repeat(out('followedBy').limit(2)).times(2).values('name’)
> gremlin>
> However, the following unrolled traversal without repeat() produces 2
> results:
> gremlin>
> g.V().has('name','JAM').out('followedBy').limit(2).out('followedBy').limit(2).propertyMap('name’)
> ==>[name:[vp[name->HURTS ME TOO]]]
> ==>[name:[vp[name->BLACK THROATED WIND]]]
> These examples demonstrate how having globally tracked limit() and range()
> counters inside repeat() are counter intuitive and instead should be tracked
> per-iteration.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
