You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by okram <gi...@git.apache.org> on 2017/11/15 16:15:13 UTC
[GitHub] tinkerpop pull request #748: TINKERPOP-1834: Consider iterate() as a first c...
GitHub user okram opened a pull request:
https://github.com/apache/tinkerpop/pull/748
TINKERPOP-1834: Consider iterate() as a first class step
https://issues.apache.org/jira/browse/TINKERPOP-1834
When a user triggers a "terminal method" (e.g. `toList()`, `toSet()`, `iterate()`), that method is now appended to the traversal's `Bytecode`. This allows providers to know what the user's intention is for the results of the traversal. This can allow them to make internal optimizations about how to execute the traversal. The drawback of this is that `Translators` need to know about which steps are "terminal" and to avoid applying them during translation. To make this easy, `Translator.TERMINAL_STEPS` is a `public static final` field to allow easy introspection.
VOTE +1.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1834
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/748.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #748
----
commit 3adfe7ec66d8cf24f3e8090ffe8dd557600b5b6c
Author: Marko A. Rodriguez <ok...@gmail.com>
Date: 2017-11-15T16:11:26Z
Added terminal method to the Traversal bytecode so providers know what the user used to trigger the evaluation.
----
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
```
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:30 h
[INFO] Finished at: 2017-11-15T12:13:52-07:00
[INFO] Final Memory: 164M/1526M
[INFO] ------------------------------------------------------------------------
```
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
A `cap()` step is a `SupplyBarrierStep` and thus, requires an emission -- a single emission, but an emission nonetheless. The ultimate solution step has to a `FilterStep` by nature so it can truly return "nothing." However, we are now getting into particulars that don't need arguing. The concept is what matters. Now that I get the problem, you simply don't want to have to "reattach" and send data back if you don't have to, then the solution is, filter everything as the last step. That is, have `iterate()` append to the bytecode a "filter all." Easy. How that is done, just needs some fiddlin' around.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
Closing. Going to provide a simpler solution.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the issue:
https://github.com/apache/tinkerpop/pull/748
`filter(false)` == `not(identity())`
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/748
Unless I'm not thinking of something, this approach won't work for remote traversals. When you call iterate on the client, it will serialize to bytecode. The bytecode ends up on the server and then becomes deserialized back into a traversal. This server traversal will not know that the client called `iterate()` and then won't be able to also call `iterate()` on the server side - which goes back to the optimization I mentioned in my previous comment.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/748
In reviewing this, I think Gremlin Server needs a change. When we submit a remote traversal. Gremlin Server needs to check for `iterate()` in the bytecode and actually call `iterate()` rather than `next()` that way the graph instance gets the right method called on it for any additional optimization that could occur there. Plus, if the client did call `iterate()` it's better that Gremlin Server simply return no results - that's a big optimization. I can work on that change and just commit it to this branch as part of this PR.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
Yes, `filter(false)` is just "the concept." The step would be:
```
filter(FalseTraversal.instance())
```
Or as @dkuppitz says -- `not(identity())`.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/748
I think I like @BrynCooke suggestion. an empty `cap()` currently generates an error so this might be a good use for it. being equivalent to `iterate()` seems logical to me.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
I think we over engineered this ticket. I believe @dkuppitz has the best idea.
```
public Traversal<S,E> iterate() {
this.filter(false);
while(hasNext()) {
next();
}
return this;
}
```
---
[GitHub] tinkerpop pull request #748: TINKERPOP-1834: Consider iterate() as a first c...
Posted by okram <gi...@git.apache.org>.
Github user okram closed the pull request at:
https://github.com/apache/tinkerpop/pull/748
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
Makes sense.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/748
There is nothing for "the server" to know. Thus, gutting the introspection. By appending `filter(false)` to the bytecode, you have a traversal that returns nothing. Which is exactly what you want. No need for the receiving/executing engine to reason about anything. A 3-line change to `iterate()` would do the trick.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by BrynCooke <gi...@git.apache.org>.
Github user BrynCooke commented on the issue:
https://github.com/apache/tinkerpop/pull/748
Would a cap step with no side effect keys be a better fit here rather than filter?
It would remove the need for a 'false' traversal.
---
[GitHub] tinkerpop issue #748: TINKERPOP-1834: Consider iterate() as a first class st...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/748
oh....`filter(false)` referred to adding a `FilterStep` to the end of the traversal basically that didn't allow any traversers through. Of course, a provider writing a `TraversalStrategy` wouldn't be able to detect "false" if you use the a lambda in `filter()`, so the provider couldn't optimize properly. Are you thinking something else here?
---