[ 
https://issues.apache.org/jira/browse/TINKERPOP-3207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18033278#comment-18033278
 ] 

ASF GitHub Bot commented on TINKERPOP-3207:
-------------------------------------------

Cole-Greer commented on code in PR #3254:
URL: https://github.com/apache/tinkerpop/pull/3254#discussion_r2464420735


##########
gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupCountTest.java:
##########
@@ -237,6 +238,7 @@ public void 
g_V_unionXoutXknowsX__outXcreatedX_inXcreatedXX_groupCount_selectXva
 
     @Test
     @LoadGraphWith(MODERN)
+    @Ignore("There is a bug in OLAP which prevents LocalBarrier steps from 
following cap() in a traversal.")

Review Comment:
   Note: I seem to have stumbled into a longstanding bug in OLAP with this 
change, as this `LocalBarrier->Cap->LocalBarrier->Cap` traversal structure 
results in workers setting global side effects during execution which is not 
permitted. This sort of query structure previously failed with AggregateGlobal, 
and is now failing with GroupCount as well due to the addition of LocalBarrier.
   
   ```
   
g.V().both().aggregate("a").out().cap("a").unfold().both().aggregate("b").cap("b")
   java.lang.RuntimeException: java.lang.IllegalStateException: 
java.lang.IllegalArgumentException: The memory can only be set() during vertex 
program setup and terminate: a
   ```





> Add Barriers to GroupSideEffect, GroupCountSideEffect, TreeSideEffect, and 
> Subgraph Steps
> -----------------------------------------------------------------------------------------
>
>                 Key: TINKERPOP-3207
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-3207
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: process
>    Affects Versions: 3.7.4
>            Reporter: Cole Greer
>            Priority: Major
>
> The gremlin language is mostly agnostic as to if a traversal should be 
> executed lazily (DFS), or eagerly (BFS). Our documentation does not specify a 
> preference for either method, and most traversals will give the same results 
> regardless of evaluation order.
> There are currently 5 steps which may lead to different results if evaluated 
> in different orders: AggregateLocalStep, GroupSideEffectStep, 
> GroupCountSideEffectStep, TreeSideEffectStep, and SubgraphStep. 
> Consider the following example:
> {code:java}
> // Lazy evaluation
> gremlin> g.V().groupCount(“x”).select(“x”)
> ==>[v[1]:1]
> ==>[v[1]:1,v[2]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> //Eager evaluation
> gremlin> g.V().groupCount(“x”).select(“x”)
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> {code}
> This inconsistency can be resolved by pulling the solution from 
> AggregateGlobalStep and making each of these steps LocalBarrier's. If there 
> are ever cases where users need the old "one-at-a-time" accumulation, this 
> will still be achievable by embedding the step inside local().
> {code:java}
> gremlin> g.V().local(groupCount("x")).select("x")
> ==>[v[1]:1]
> ==>[v[1]:1,v[2]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1]
> ==>[v[1]:1,v[2]:1,v[3]:1,v[4]:1,v[5]:1,v[6]:1]
> {code}
> This will ensure that all traversals will return consistent results, 
> regardless of if is is run in a lazy or eager traversal engine.
> AggregateLocalStep is excluded from these changes as AggregateGlobalStep 
> already implements the desired fix, and AggregateLocalStep is slated for 
> removal according to the [proposal on Lazy/Eager 
> evaluation|https://github.com/apache/tinkerpop/blob/2c3f31fdab535913c5a7b318e16f1c80bce57f35/docs/src/dev/future/proposal-scoping-5.asciidoc#proposed-further-simplifications].



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to