Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-2626

RangeGlobalStep closes traversal prematurely

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 3.4.12, 3.5.1
    • 3.6.0, 3.4.13, 3.5.2
    • process
    • None

    Description

      In the RangeGlobalStep.java, this line assumes that RangeGlobalStep will be the last step in the traversal. Due to this assumption, it closes the traversal when it has no results to share.

      This assumption is not true since a limit() step can be put anywhere in the middle of the query e.g. `g.V().limit(2).out()`

      Due to this behaviour, a query such as `g.V().limit(2).aggregate('x').cap('x').unfold().out()` will execute the last out() step with a "closed traversal". Let's dry run this query:

      The traversal blocks at aggregate barrier step and tries to fetch all possible incoming solutions till the aggregate step. limit() will provide 2 solutions and on the third solution, it will close the traversal and return a NoElementFoundException to downstream steps. aggregate() will consider NoElementFoundException as a signal for "no more upstream results" and will aggregate the two solutions. These two solutions will be forwarded to out() step for computation. BUT limit() has already closed the traversal. And hence, out() should not be able to execute.

      This inconsistency is not caught in the tests because the tests are run against TinkerGraph which will work even if traversal has been closed, hence, out() step works even if no underlying iterators are present.

      Solution:
      1. Remove the line that closes the traversal here: https://github.com/apache/tinkerpop/blame/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/RangeGlobalStep.java#L68 (along with the incorrect comment)
      2. Add a lifecycle state in the steps. Every step should validate that it is not closed before execution of each result. If it is closed, steps should throw a IllegalStateException.
      3. Enhance TinkerGraph to add a concept of open/closed iterators. Ideally, since the iterators were closed on traversal close, out() should not have processed successfully  in our test cases 

      Attachments

        Activity

          People

            spmallette Stephen Mallette
            divijvaidya Divij Vaidya
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: