Github user GCHQResearcher1337 commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/876#discussion_r195046349
  
    --- Diff: 
gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatTest.java
 ---
    @@ -386,5 +456,26 @@ public void 
g_V_hasXname_markoX_repeatXoutE_inV_simplePathX_untilXhasXname_rippl
             public Traversal<Vertex, Path> 
get_g_V_hasXloop_name_loopX_repeatXinX_timesX5X_path_by_name() {
                 return 
g.V().has("loops","name","loop").repeat(__.in()).times(5).path().by("name");
             }
    +
    +        @Override
    +        public Traversal<Vertex, Path> 
get_g_V_repeatXout_repeatXoutX_timesX1XX_timesX1X_limitX1X_path_by_name() {
    +            // NB We need to prevent the RepeatUnrollStrategy from 
applying to properly exercise this test as this traversal can be simplified
    --- End diff --
    
    Thanks @spmallette I wanted to do something like this but hadn't spotted 
these tests. While writing some tests I've noticed that when 
RepeatUnrollStrategy is applied from the inside out, adding a barrier step onto 
the end: 
    repeat(a.repeat(b).times(1)).times(1)  -> repeat(a.b.barrier).times(1)
    repeat(a.b.barrier).times(1) -> a.b.barrier.barrier
    
    eg
    ```
    gremlin> g.V().repeat(out().repeat(out()).times(1)).times(1).explain()
    ==>Traversal Explanation
    
===========================================================================================================================================================================================================================
    Original Traversal                 [GraphStep(vertex,[]), 
RepeatStep([VertexStep(OUT,vertex), RepeatStep([VertexStep(OUT,vertex), 
RepeatEndStep],until(loops(1)),emit(false)), 
RepeatEndStep],until(loops(1)),emit(false))]
    
    ConnectiveStrategy           [D]   [GraphStep(vertex,[]), 
RepeatStep([VertexStep(OUT,vertex), RepeatStep([VertexStep(OUT,vertex), 
RepeatEndStep],until(loops(1)),emit(false)), 
RepeatEndStep],until(loops(1)),emit(false))]
    MatchPredicateStrategy       [O]   [GraphStep(vertex,[]), 
RepeatStep([VertexStep(OUT,vertex), RepeatStep([VertexStep(OUT,vertex), 
RepeatEndStep],until(loops(1)),emit(false)), 
RepeatEndStep],until(loops(1)),emit(false))]
    FilterRankingStrategy        [O]   [GraphStep(vertex,[]), 
RepeatStep([VertexStep(OUT,vertex), RepeatStep([VertexStep(OUT,vertex), 
RepeatEndStep],until(loops(1)),emit(false)), 
RepeatEndStep],until(loops(1)),emit(false))]
    InlineFilterStrategy         [O]   [GraphStep(vertex,[]), 
RepeatStep([VertexStep(OUT,vertex), RepeatStep([VertexStep(OUT,vertex), 
RepeatEndStep],until(loops(1)),emit(false)), 
RepeatEndStep],until(loops(1)),emit(false))]
    IncidentToAdjacentStrategy   [O]   [GraphStep(vertex,[]), 
RepeatStep([VertexStep(OUT,vertex), RepeatStep([VertexStep(OUT,vertex), 
RepeatEndStep],until(loops(1)),emit(false)), 
RepeatEndStep],until(loops(1)),emit(false))]
    AdjacentToIncidentStrategy   [O]   [GraphStep(vertex,[]), 
RepeatStep([VertexStep(OUT,vertex), RepeatStep([VertexStep(OUT,vertex), 
RepeatEndStep],until(loops(1)),emit(false)), 
RepeatEndStep],until(loops(1)),emit(false))]
    RepeatUnrollStrategy         [O]   [GraphStep(vertex,[]), 
VertexStep(OUT,vertex), VertexStep(OUT,vertex), NoOpBarrierStep(2500), 
NoOpBarrierStep(2500)]
    CountStrategy                [O]   [GraphStep(vertex,[]), 
VertexStep(OUT,vertex), VertexStep(OUT,vertex), NoOpBarrierStep(2500), 
NoOpBarrierStep(2500)]
    PathRetractionStrategy       [O]   [GraphStep(vertex,[]), 
VertexStep(OUT,vertex), VertexStep(OUT,vertex), NoOpBarrierStep(2500), 
NoOpBarrierStep(2500)]
    LazyBarrierStrategy          [O]   [GraphStep(vertex,[]), 
VertexStep(OUT,vertex), NoOpBarrierStep(2500), VertexStep(OUT,vertex), 
NoOpBarrierStep(2500), NoOpBarrierStep(2500)]
    TinkerGraphCountStrategy     [P]   [GraphStep(vertex,[]), 
VertexStep(OUT,vertex), NoOpBarrierStep(2500), VertexStep(OUT,vertex), 
NoOpBarrierStep(2500), NoOpBarrierStep(2500)]
    TinkerGraphStepStrategy      [P]   [TinkerGraphStep(vertex,[]), 
VertexStep(OUT,vertex), NoOpBarrierStep(2500), VertexStep(OUT,vertex), 
NoOpBarrierStep(2500), NoOpBarrierStep(2500)]
    ProfileStrategy              [F]   [TinkerGraphStep(vertex,[]), 
VertexStep(OUT,vertex), NoOpBarrierStep(2500), VertexStep(OUT,vertex), 
NoOpBarrierStep(2500), NoOpBarrierStep(2500)]
    StandardVerificationStrategy [V]   [TinkerGraphStep(vertex,[]), 
VertexStep(OUT,vertex), NoOpBarrierStep(2500), VertexStep(OUT,vertex), 
NoOpBarrierStep(2500), NoOpBarrierStep(2500)]
    
    Final Traversal                    [TinkerGraphStep(vertex,[]), 
VertexStep(OUT,vertex), NoOpBarrierStep(2500), VertexStep(OUT,vertex), 
NoOpBarrierStep(2500), NoOpBarrierStep(2500)]
    ```
    
    Do you think it's ok to leave multiple barrier steps at the end? I could 
add an extra check that we aren't inserting a barrier after a barrier here - 
but this could interfere with a barrier added by a user?
    
https://github.com/apache/tinkerpop/blob/d3b6a340dbb9c89355c40089a5c083dc0dbd4c5b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/RepeatUnrollStrategy.java#L72


---

Reply via email to