Details
-
Improvement
-
Status: Closed
-
Major
-
Resolution: Fixed
-
3.0.1-incubating
-
None
Description
We should allow mid-traversal V() and E(). It shouldn't be a technical problem, since we always know the traversal source, thus it's just a simple flatMap. Vendors could then implement mid-traversal global index lookups.
Example:
g.V().hasLabel("company").has("name", "DataStax").in("worksFor").as("dse"). V().hasLabel("company").has("name", "Aurelius").in("worksFor").as("ae"). addOutE("dse", "knows", "ae", "since", "02/2015")
Attachments
Issue Links
- is related to
-
TINKERPOP-2798 Add support for mid-traversal E()
- Closed
Activity
What about a structure that could be used as a source, and that would allow the intent to be more obvious, e.g. pairwise vs. cross-product, etc. using logic like match?
It seems preferable to allow initiation of this from a source, so that the strategy can decide which order to execute the two or more source traversals.
Here is a "join" reasoning traversal, creating edges based on property matching:
g.match( __.V().hasLabel('company').as('c'), __.V().hasLabel('person').as('p'), __.as('c').values('name').as('cname'), __.as('p').values('companyName').as('pname')) .where('cname', eq('pname') .dedup('c', 'p') .select('c', 'p') .addOutE('p', 'worksFor', 'c')
Here is a "cross-product" traversal, materializing the NxN space, except the diagonal:
g.match( __.V().hasLabel('person').as('a'), __.V().hasLabel('person').as('b')) .where('a', neq('b')) .select('a', 'b').as('pair') .addOutE('a', 'relatesTo', 'b') .select('pair').addOutE('b', 'relatesTo', 'a')
For each incoming object; it's just a simple flatMap.
mhfrantz: You won't need those complicated match patterns:
Query 1:
g.V().hasLabel('company').as('c','cv'). V().hasLabel('person').as('p','pv').dedup('c','p'). select('c','p').by('cname').by('pname').where('c', eq('p')). addOutE('pv', 'worksFor', 'cv')
Query 2:
g.V().hasLabel('person').as('a'). V().hasLabel('person').as('b'). where('a', neq('b')). addOutE('a', 'relatesTo', 'b'). addOutE('b', 'relatesTo', 'a')
I was thinking that the match step logic would allow the merge of multiple sources to be dynamically optimized, and would properly capture the symmetry of a multi-source traversal. If written as a mid-traversal step, the author must decide which traversal is the "outer loop" and which the "inner loop", or else we rely on a strategy that splits and rewrites the traversal using the match step logic.
So perhaps we can start with the imperative mid-traversal step (this JIRA issue), and then work toward allowing match to optimize multiple sources (a separate JIRA issue).
This is in the TINKERPOP3-763 branch.
Open items:
- Do we add E() support?
- Add more test cases to GraphTest.
- Add Graph Step to the documentation – weird.
dkuppitz – if you want to handle all 3, please do. If you just do the test cases, ping me when you are done and I can do the docs and E()-support. However, we should decide wether we want E() and if we do, can you just add it cause you will need to test it.
Oh – and dkuppitz – can you also make a GraphStepTest that does that hashCode() stuff you do well. In fact, I think we might need a hashCode() method in GraphStep as it no longer extends StartStep. And we need a reset method.
Given the comments in https://issues.apache.org/jira/browse/TINKERPOP3-902, I can just say that I don't want/need E() as a start step nor as a mid-traversal step.
dkuppitz Please do the following.
1. Add more test cases to GraphStep.
2. Write the documentation for "Graph Step" (include CAUTION: about indicies).
3. Write GraphStepTest to verify hashCode() (and you may need to make a hashCode() impl for GraphStep.
4. Please write TinkerGraphStepStrategyTest and Neo4jGraphStepStrategyTest to ensure they you are getting "has container fold in."
To point 4, you can probably just copy/paste the test case for TinkerGraphStepStrategyTest to Neo4jGraphStepStrategyTest as the implementation for both is nearly identical. Though, be sure we have both tests as they may evolve independently.
I just pushed all my changes. You'll probably want to extend the docs section.
You did not complete your work correctly and in full. Please see my comment yesterday on what is required of you for this ticket.
GitHub user dkuppitz opened a pull request:
https://github.com/apache/incubator-tinkerpop/pull/125
TINKERPOP3-762: Allow mid-traversal V() (and E())
- @okram added `V()` as a valid mid-traversal step
- added traversal test cases
- added integration tests to verify that indexes will be used in `TinkerGraph` and `Neo4jGraph`
- added a new `GraphStep` section in the docs
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-762
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tinkerpop/pull/125.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 #125
commit 5c5bd06ebbd4ad6781ba46349ebb638f93b4f469
Author: Marko A. Rodriguez <okrammarko@gmail.com>
Date: 2015-10-20T20:09:03Z
GraphStep no longer extends StartStep. It now takes a boolean isStart in its constructor. Moreover, it is no longer a SideEffectStep but a FlatMapStep so it moved to the map/ package. TinkerGraphStepStrateagy and Neo4jGRaphStepStrategy had to be updated to look for ALL GraphStep instances in the traversal and optimize them. Note that @spmallette work in ElementIdStrategy, PartitionStrategy, etc. already did 'for all graph steps' so that didnt need updating. Updated __ and GraphTraversal with V(). I added a new test class called GraphTest with one simple test. I'm passing this off to @dkuppitz to populate GraphTest with compliacated examples. Also, we need to decided if we want mid-traversal E().
commit 0841bbdc009e4af20fb8bff7899b296eaad3c4da
Author: Marko A. Rodriguez <okrammarko@gmail.com>
Date: 2015-10-20T20:26:50Z
added a reset() step.
commit 7dd62126c67ade1b59016ab017e6bec7b8e70cc1
Author: Marko A. Rodriguez <okrammarko@gmail.com>
Date: 2015-10-20T21:34:06Z
Merge branch 'master' into TINKERPOP3-762
commit 6c0e0bd9d0ade9ca62cf1f2a6bdb7a5a3ad0293e
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-25T15:15:09Z
added a more complex mid-traversal V() traversal test
commit 7b18447d3641811c291c309180d5688642a8a053
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-27T14:47:02Z
added another GraphStep traversal test
commit c35d08f255f4cdd4d498765291a08748423db460
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-27T14:47:42Z
implemented GraphStep::hashCode()
commit d83bf71b6ace35f3f0cdf582ba95a67b595fac27
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-27T14:48:44Z
added a GraphStep core test (hashCode() verification)
commit a976ca672456590999dfd8d26c7ee27ed558e572
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-27T16:33:01Z
verified that TinkerGraph and Neo4jGraph fold HasContainers into GraphStep
commit df443cc5f71904c167ce86149d4275fc8ea6aa67
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-28T11:39:23Z
added a docs section for GraphStep
commit 402e57d94fd71f2d652aded37becfe566d4ddad4
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-28T14:32:26Z
added a srategy test suite for Neo4j
commit 15d6e7a47bac6609dd890635b568ca05e7862bf0
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-28T14:33:05Z
Revert "verified that TinkerGraph and Neo4jGraph fold HasContainers into GraphStep"
This reverts commit a976ca672456590999dfd8d26c7ee27ed558e572.
commit a71a17983d389be2261a2cf12398c485a25813a7
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-28T15:06:30Z
mirrored approach to test strategies from neo4j-gremlin into tinkergraph-gremlin
commit 0900d5f6875ef278a20e6512c1b0f7b77d8586b2
Author: Daniel Kuppitz <daniel_kuppitz@hotmail.com>
Date: 2015-10-28T15:16:22Z
updated CHANGELOG and release doc
Github user okram commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/125#issuecomment-151895846
Ran `mvn clean install` and it worked. +1. Also, I review the test suite developments, docs, and did the mid-traversal `V()` implementation myself.
Github user dkuppitz commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/125#issuecomment-152013958
All the tests there were implemented specifically for the changes in this PR pass for me. However, I'm currently having issues with Giraph and can't get the full integration test suite to pass.
Hence, if anybody else can confirm, that integration tests still pass, my vote is +1.
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/125#issuecomment-152231933
code looks tested, docs look good, and `mvn clean install` works on my end - if that is sufficient testing/review then i'm +1
So, does it work such that for each incoming object, V(). Or, we do a NullReducingBarrier and then V()?