You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "vkagamlyk (via GitHub)" <gi...@apache.org> on 2023/03/15 18:32:08 UTC

[GitHub] [tinkerpop] vkagamlyk opened a new pull request, #1992: [TINKERPOP-2824] Properties on Elements (#63)

vkagamlyk opened a new pull request, #1992:
URL: https://github.com/apache/tinkerpop/pull/1992

   - Changed default behavior for returning properties on Elements for OLTP queries. Properties now returned.
   - Detachment is no longer performed in `TraverserIterator`.
   - Added `materializeProperties` request option to control properties serialization.
   - Modified serializers in to handle serialization and deserialization of properties.
   - Added functional properties to the graph structure components for .NET, GO and Python.
   - Modified the GremlinScriptChecker to extract the `materializeProperties` request option.
   - Neo4jVertexProperty no longer throw Exception for `properties()`, but return empty iterable.
   
   Jira ticket: https://issues.apache.org/jira/browse/TINKERPOP-2824
   


-- 
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


[GitHub] [tinkerpop] vkagamlyk commented on pull request #1992: [TINKERPOP-2824] Properties on Elements (#63)

Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #1992:
URL: https://github.com/apache/tinkerpop/pull/1992#issuecomment-1473149288

   VOTE +1


-- 
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


[GitHub] [tinkerpop] xiazcy commented on a diff in pull request #1992: [TINKERPOP-2824] Properties on Elements (#63)

Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #1992:
URL: https://github.com/apache/tinkerpop/pull/1992#discussion_r1137732816


##########
docs/src/dev/provider/index.asciidoc:
##########
@@ -194,7 +194,8 @@ The following bullets provide some tips to consider when implementing the struct
 * `VertexProperty`
 ** This interface is both a `Property` and an `Element` as `VertexProperty` is a first-class graph element in that it
 can have its own properties (i.e. meta-properties). Even if the implementation does not intend to support
-meta-properties, the `VertexProperty` needs to be implemented as an `Element`.
+meta-properties, the `VertexProperty` needs to be implemented as an `Element`. `VertexProperty` should return empty
+iterable for properties if not support meta-properties.

Review Comment:
   ```suggestion
   iterable for properties if meta-properties is not supported.
   ```



##########
docs/src/upgrade/release-3.7.x.asciidoc:
##########
@@ -29,6 +29,74 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.7.0/CHANGELOG.asc
 
 === Upgrading for Users
 
+==== Properties on Elements
+
+===== Introduction
+
+By default properties on `Element` are now returned for OLTP requests. Gremlin Server 3.5 and 3.6 can return properties only in some special cases. 
+More history details about serialization of properties can be found in the link:https://lists.apache.org/thread/xltcon4zxnwq4fyw2r2126syyrqm8spy[Stephen's post].
+
+===== Behavior for OLAP queries
+
+Queries still won't return properties on Elements. The main reason for this is performance considerations.
+If you need to get a property, then this can be explicitly configured with `HaltedTraverserStrategy`
+
+[source,java]
+----
+  g.withComputer().withStrategies(HaltedTraverserFactoryStrategy.detached())
+----
+
+===== Output comparison for Gremlin Server 3.5/3.6 and 3.7
+
+Let's take a closer look at a Javascript GLV code example in 3.6 and 3.7:
+
+[source,javascript]
+----
+  const client = new Client('ws://localhost:8182/gremlin',{traversalSource: 'gmodern'});
+  await client.open();
+  const result = await client.submit('g.V(1)');
+  console.log(JSON.stringify(result.first()));
+  await client.close();
+----
+
+The result will be different depending on the version of Gremlin Server.
+For 3.5/3.6:
+[source,json]
+----
+  {"id":1,"label":"person"}
+----
+
+For 3.7:
+[source,json]
+----
+  {"id":1,"label":"person","properties":{"name":[{"id":0,"label":"name","value":"marko","key":"name"}],"age":[{"id":1,"label":"age","value":29,"key":"age"}]}}
+---- 
+
+===== Enabling the previous behavior
+
+The GLVs in 3.5/3.6 will not be able to work correctly with properties on Elements. If you don't need to get properties then you can do one of the following:
+
+* To configure Gremlin Server to not return properties, update Gremlin Server initialization script with `ReferenceElementStrategy`.
+This method is better to use with 3.5/3.6 GLVs.
+For example 
+[source,groovy]
+----
+globals << [g : traversal().withEmbedded(graph).withStrategies(ReferenceElementStrategy)]
+----
+
+* Use config per request `with('materializeProperties', 'tokens')`
+[source,csharp]
+----
+g.With("materializeProperties", "tokens").V(1).Next()
+----
+
+===== Possible issues
+
+ReferenceElement-type objects are no longer returned - you get a DetachedElement from remote requests. If you not have been coding to the `Element` interfaces then need to update the code with use the interfaces like `Vertex` and `Edge`.

Review Comment:
   ```suggestion
   ReferenceElement-type objects are no longer returned - you get a DetachedElement from remote requests. If you have not been implementing the `Element` interfaces then you will need to update the code to use interfaces like `Vertex` and `Edge`.
   ```



##########
CHANGELOG.asciidoc:
##########
@@ -38,6 +38,13 @@ This release also includes changes from <<release-3-6-XXX, 3.6.XXX>>.
 * Reduces dependency from `gremlin-server` onto `gremlin-driver` to a test scope only.
 * Added `RequestOptions` and `RequestOptionsBuilder` types to Go GLV to encapsulate per-request settings and bindings.
 * Added `SubmitWithOptions()` methods to `Client` and `DriverRemoteConnection` in Go GLV to pass `RequestOptions` to the server.
+* Changed default behavior for returning properties on Elements for OLTP queries. Properties now returned.

Review Comment:
   ```suggestion
   * Changed default behavior for returning properties on Elements for OLTP queries. Properties are now returned.
   ```



-- 
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


[GitHub] [tinkerpop] spmallette commented on pull request #1992: [TINKERPOP-2824] Properties on Elements (#63)

Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on PR #1992:
URL: https://github.com/apache/tinkerpop/pull/1992#issuecomment-1472644450

   Assuming there are tests for all language variants that cover all cases where you can have properties on elements, thus: `Vertex`, `Edge` and `VertexProperty` then: 
   
   VOTE +1 - final answer 🚢 


-- 
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


[GitHub] [tinkerpop] codecov-commenter commented on pull request #1992: [TINKERPOP-2824] Properties on Elements (#63)

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1992:
URL: https://github.com/apache/tinkerpop/pull/1992#issuecomment-1470607251

   ## [Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1992](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a466272) into [master](https://codecov.io/gh/apache/tinkerpop/commit/1b667b6e25fb32e98d33f8d902ab2303a69030aa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1b667b6) will **increase** coverage by `0.44%`.
   > The diff coverage is `69.36%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1992      +/-   ##
   ============================================
   + Coverage     68.55%   69.00%   +0.44%     
   - Complexity     9091     9107      +16     
   ============================================
     Files           855      830      -25     
     Lines         41276    37556    -3720     
     Branches       5603     5625      +22     
   ============================================
   - Hits          28298    25914    -2384     
   + Misses        11005     9819    -1186     
   + Partials       1973     1823     -150     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...he/tinkerpop/gremlin/structure/VertexProperty.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvVmVydGV4UHJvcGVydHkuamF2YQ==) | `40.00% <0.00%> (ø)` | |
   | [.../gremlin/driver/remote/DriverRemoteConnection.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1kcml2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL2RyaXZlci9yZW1vdGUvRHJpdmVyUmVtb3RlQ29ubmVjdGlvbi5qYXZh) | `47.91% <0.00%> (-1.02%)` | :arrow_down: |
   | [...p/gremlin/neo4j/structure/Neo4jVertexProperty.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bmVvNGotZ3JlbWxpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvdGlua2VycG9wL2dyZW1saW4vbmVvNGovc3RydWN0dXJlL05lbzRqVmVydGV4UHJvcGVydHkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...lin/server/handler/HttpGremlinEndpointHandler.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9oYW5kbGVyL0h0dHBHcmVtbGluRW5kcG9pbnRIYW5kbGVyLmphdmE=) | `40.66% <27.27%> (-1.48%)` | :arrow_down: |
   | [...a/org/apache/tinkerpop/gremlin/server/Context.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9Db250ZXh0LmphdmE=) | `74.19% <50.00%> (-8.91%)` | :arrow_down: |
   | [...pache/tinkerpop/gremlin/driver/RequestOptions.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1kcml2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL2RyaXZlci9SZXF1ZXN0T3B0aW9ucy5qYXZh) | `76.47% <60.00%> (-1.80%)` | :arrow_down: |
   | [...in/structure/io/binary/types/VertexSerializer.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vYmluYXJ5L3R5cGVzL1ZlcnRleFNlcmlhbGl6ZXIuamF2YQ==) | `84.61% <75.00%> (-15.39%)` | :arrow_down: |
   | [...mlin/structure/io/binary/types/EdgeSerializer.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vYmluYXJ5L3R5cGVzL0VkZ2VTZXJpYWxpemVyLmphdmE=) | `91.30% <80.00%> (-8.70%)` | :arrow_down: |
   | [.../gremlin/structure/util/detached/DetachedEdge.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvdXRpbC9kZXRhY2hlZC9EZXRhY2hlZEVkZ2UuamF2YQ==) | `87.93% <81.81%> (-1.44%)` | :arrow_down: |
   | [...tructure/util/detached/DetachedVertexProperty.java](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvdXRpbC9kZXRhY2hlZC9EZXRhY2hlZFZlcnRleFByb3BlcnR5LmphdmE=) | `94.44% <85.71%> (-1.56%)` | :arrow_down: |
   | ... and [7 more](https://codecov.io/gh/apache/tinkerpop/pull/1992?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ... and [41 files with indirect coverage changes](https://codecov.io/gh/apache/tinkerpop/pull/1992/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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


[GitHub] [tinkerpop] vkagamlyk merged pull request #1992: [TINKERPOP-2824] Properties on Elements (#63)

Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk merged PR #1992:
URL: https://github.com/apache/tinkerpop/pull/1992


-- 
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