[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request #1555: TINKERPOP-2681 mergeE() and mergeV()

[GitBox <gi...@apache.org>]

FlorianHockmann commented on a change in pull request #1555:
URL: https://github.com/apache/tinkerpop/pull/1555#discussion_r797389596



##########
File path: docs/src/upgrade/release-3.6.x.asciidoc
##########
@@ -29,6 +29,71 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.6.0/CHANGELOG.asc
 
 === Upgrading for Users
 
+==== mergeV() and mergeE()
+
+One of the most commonly used patterns in Gremlin is the use of `fold().coalesce(unfold(), ...)` to perform upsert-like
+functionality. While this pattern is quite flexible, it can also be confusing to new users and for certain use cases
+challenging to get the pattern correctly implemented. For providers, the pattern is difficult to properly optimize
+because it can branch into complexity quite quickly making it hard to identify a section of Gremlin for an upsert and
+therefore is not executed as efficiently as it might have been otherwise.
+
+The new `mergeV()` and `mergeE()` steps greatly simplify this pattern and as the pattern is condensed into a single
+step it should be straightforward for providers to optimize as part of their implementations. The following example
+demonstrates just how much easier implementing a basic upsert of a vertex has gotten:
+
+[source,text]
+----
+// prior to 3.6.0, use fold().coalesce(unfold(), ...)
+gremlin> g.V().has('person','name','vadas').has('age', 27).
+......1>   fold().
+......2>   coalesce(unfold().property('age',30),
+......3>            addV('person').property('name','vadas').property('age',27)).

Review comment:
       I find the example here already quite advanced and not obvious to understand. It took me at least a bit to understand that it's really intended here to create a vertex with `age=27` if it doesn't exist already and to change its age otherwise to `30`.
   
   I suggest that we use a simpler example here in the upgrade docs and I guess most current usages of the fold/coalesce pattern out there are more of the nature:
   
   ```
   g.V().
     has('person', 'name', 'vadas').
     fold().
     coalesce(unfold(), addV('person').property('name', 'vadas')).
     property('age', 30)
   ```
   
   If I understand the new functionality correctly, then this doesn't need any `option()` at all, right? We could then afterwards maybe simply mention these additional options and then either link to the reference docs where they are explained in more details or add additional more advanced examples here that show these options.

##########
File path: CHANGELOG.asciidoc
##########
@@ -27,6 +27,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Changed TinkerGraph to allow identifiers to be heterogeneous when filtering.
 * Prevented values of `T` to `property()` from being `null`.
 * Added `fail()` step.
+* Added `mergeV()` and `mergeE()` steps.
+* Moved `TraversalOptionParent.Pick` to it's own class as `Pick`.

Review comment:
       ```suggestion
   * Moved `TraversalOptionParent.Pick` to its own class as `Pick`.
   ```

##########
File path: docs/src/upgrade/release-3.6.x.asciidoc
##########
@@ -29,6 +29,71 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.6.0/CHANGELOG.asc
 
 === Upgrading for Users
 
+==== mergeV() and mergeE()
+
+One of the most commonly used patterns in Gremlin is the use of `fold().coalesce(unfold(), ...)` to perform upsert-like
+functionality. While this pattern is quite flexible, it can also be confusing to new users and for certain use cases
+challenging to get the pattern correctly implemented. For providers, the pattern is difficult to properly optimize
+because it can branch into complexity quite quickly making it hard to identify a section of Gremlin for an upsert and
+therefore is not executed as efficiently as it might have been otherwise.
+
+The new `mergeV()` and `mergeE()` steps greatly simplify this pattern and as the pattern is condensed into a single
+step it should be straightforward for providers to optimize as part of their implementations. The following example
+demonstrates just how much easier implementing a basic upsert of a vertex has gotten:
+
+[source,text]
+----
+// prior to 3.6.0, use fold().coalesce(unfold(), ...)
+gremlin> g.V().has('person','name','vadas').has('age', 27).
+......1>   fold().
+......2>   coalesce(unfold().property('age',30),
+......3>            addV('person').property('name','vadas').property('age',27)).
+......4>   elementMap()
+==>[id:2,label:person,name:vadas,age:30]
+
+// 3.6.0
+gremlin> g.mergeV([(T.label): 'person', name:'vadas', age: 27]).
+......1>     option(onMatch, [age: 30]).
+......2>   elementMap()
+==>[id:2,label:person,name:vadas,age:30]
+----
+
+The `fold().coalesce(unfold(), ...)` pattern was even more complicated for upserting edges, but the following example
+demonstrates how much easier `mergeE()` is to follow:
+
+[source,text]
+----
+// prior to 3.6.0, use fold().coalesce(unfold(), ...)

Review comment:
       (nitpick) This comment says that `fold().coalesce(unfold(), ...)` is used but the Gremlin traversal doesn't use `fold()` and `unfold()`. Maybe we should really change this traversal to also use `fold()` and `unfold()`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org