TINKERPOP-3023 Expand type syntax in grammar in 3.8 - Remove Vertex#3133
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 3.8-dev #3133 +/- ##
==========================================
Coverage ? 76.52%
Complexity ? 13829
==========================================
Files ? 1109
Lines ? 69787
Branches ? 7545
==========================================
Hits ? 53404
Misses ? 13617
Partials ? 2766 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
EchoNullify
changed the title
EchoNullify
changed the title
| g.mergeE(["weight": 0.5, Direction.out: 1]) | ||
| g.mergeE(["weight": 0.5, Direction.in: 1]) | ||
| g.addE('knows').from(g.V(1)) | ||
| g.addE('knows').from(g.V(1).next()) |
There was a problem hiding this comment.
Is this now a valid traversal?
There was a problem hiding this comment.
Previously was invalid but now works
There was a problem hiding this comment.
This is interesting. from() in the grammar previously only allowed a nestedTraversal, but not a terminatedTraversal. By putting genericArgument in there to accept any arbitrary ID type, we've now allowed terminatedTraversal to leak in. I don't think this is a big concern.
@spmallette any thoughts on this?
| g.mergeE(["weight": 0.5, Direction.out: 1]) | ||
| g.mergeE(["weight": 0.5, Direction.in: 1]) | ||
| g.addE('knows').from(g.V(1)) | ||
| g.addE('knows').from(g.V(1).next()) |
There was a problem hiding this comment.
This is interesting. from() in the grammar previously only allowed a nestedTraversal, but not a terminatedTraversal. By putting genericArgument in there to accept any arbitrary ID type, we've now allowed terminatedTraversal to leak in. I don't think this is a big concern.
@spmallette any thoughts on this?
|
I've pushed some changes into the branch which updates the docs a bit and strips out the vertex -> id sugar in GraphTraversal as it's not required for bytecode. (to be added to master in a separate PR) My vote is VOTE +1 |
|
Note: Followup PR for master: #3138 |
…3138) This is a followup to #3133 with changes exclusively for TP4. The primary difference in TP4 is that bytecode has been replaced with GremlinLang, which means that the use of Vertex in V(), from(), and to() is now sugar added by GraphTraversal. These vertices must be converted to their ids when passed into GremlinLang. --------- Co-authored-by: Peter Tribe <peter.tribe@improving.com>
from() and to() will now only accept Strings (alias for __.select()) and Traversals. If users wish to pass a vertex id, they may embed it in __.V() or __.constant(). The primary motivation here is to ensure consistent behaviour accross providers regardless of if their id types are numerics, strings, or other. This change is a followup to an unintended consequence stemming from #3133. The issue with directly allowing ID's to be passed into from/to, it creates a weird asymmetry for graphs which use Strings as their ids. For example g.addE().from(1).to(2) would work fine and create an edge from V[1] to V[2], however if the graph used String ids, the equivalent traversal (g.addE().from("a").to("b")) does not work, as the Strings are interpreted as path labels (converted to __.select("a")) instead of vertex ids. There are some ugly workarounds which can partially mitigate this issue, but overall there is no generalized fix which allows all traversals to behave the same regardless of the vertex id type. --------- Co-authored-by: andreachild <andrea.child@improving.com>
5 participants
Feat: Remove Vertex support from grammar, and for places where a vertex is required, pass in the vertex id instead at traversal construction.
The grammar currently includes inconsistencies, particularly around "type constructors" like ReferenceVertex(), which are being re-evaluated.
ReferenceVertex should be renamed simply to Vertex, as the distinction (e.g., between ReferenceVertex and DetachedVertex) is no longer meaningful or guaranteed.
The keyword new should be removed from the grammar rule for structureVertex, as it's syntactically unnecessary.
The Vertex constructor should only require an id, not a label. Labels are not utilized in practice (e.g., in g.V(Vertex)), and can default to an empty string.
Resolves: 3023,
For reference...
https://lists.apache.org/thread/ptq81160z4y4vddg4lxz2nv244x2yzhq