You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "spmallette (via GitHub)" <gi...@apache.org> on 2024/02/13 14:29:08 UTC
[PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
spmallette opened a new pull request, #2487:
URL: https://github.com/apache/tinkerpop/pull/2487
https://issues.apache.org/jira/browse/TINKERPOP-3056
Since the current traverser was only being updated onMatch when it was used as a start step doing mutations in a sideEffect() would update whatever the current traverser was. This code may have been leftover from a time when the merge steps operated more readily on the current traverser as an argument to the step.
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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487#discussion_r1488216500
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature:
##########
@@ -841,3 +841,45 @@ Feature: Step - mergeE()
"""
When iterated to list
Then the traversal will raise an error with message containing text of "Property key can not be a hidden key: ~label"
+
+ Scenario: g_V_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").property("name", "marko").as("a").
+ addV("person").property("name", "vadas").as("b").
+ addE("knows").property("weight", 1).from("a").to("b")
+ """
+ And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\", \"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+ And the traversal of
+ """
+ g.V().mergeE(xx1).
+ option(Merge.onMatch, __.sideEffect(__.property("weight", 0)).constant([:]))
+ """
+ When iterated to list
+ Then the result should have a count of 2
+ And the graph should return 2 for count of "g.V()"
+ And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"weight\",1)"
+ And the graph should return 1 for count of "g.E().hasLabel(\"knows\").has(\"weight\",0)"
+ And the graph should return 0 for count of "g.V().hasLabel(\"knows\").has(\"weight\")"
+
+ Scenario: g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").property("name", "marko").as("a").
+ addV("person").property("name", "vadas").as("b").
+ addE("knows").property("weight", 1).from("a").to("b")
+ """
+ And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\", \"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+ And the traversal of
+ """
+ g.mergeE(xx1).
+ option(Merge.onMatch, __.sideEffect(__.property("weight", 0)).constant([:]))
+ """
+ When iterated to list
+ Then the result should have a count of 1
+ And the graph should return 2 for count of "g.V()"
+ And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"weight\",1)"
+ And the graph should return 1 for count of "g.E().hasLabel(\"knows\").has(\"weight\",0)"
+ And the graph should return 0 for count of "g.V().hasLabel(\"knows\").has(\"weight\")"
Review Comment:
```suggestion
And the graph should return 0 for count of "g.V().has(\"weight\")"
```
--
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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487#issuecomment-1942092498
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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487#issuecomment-1942076338
VOTE +1
not exactly related to the current ticket, but is there same issue for `MergeV`?
--
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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487#discussion_r1488215072
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature:
##########
@@ -841,3 +841,45 @@ Feature: Step - mergeE()
"""
When iterated to list
Then the traversal will raise an error with message containing text of "Property key can not be a hidden key: ~label"
+
+ Scenario: g_V_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").property("name", "marko").as("a").
+ addV("person").property("name", "vadas").as("b").
+ addE("knows").property("weight", 1).from("a").to("b")
+ """
+ And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\", \"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+ And the traversal of
+ """
+ g.V().mergeE(xx1).
+ option(Merge.onMatch, __.sideEffect(__.property("weight", 0)).constant([:]))
+ """
+ When iterated to list
+ Then the result should have a count of 2
+ And the graph should return 2 for count of "g.V()"
+ And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"weight\",1)"
+ And the graph should return 1 for count of "g.E().hasLabel(\"knows\").has(\"weight\",0)"
+ And the graph should return 0 for count of "g.V().hasLabel(\"knows\").has(\"weight\")"
Review Comment:
```suggestion
And the graph should return 0 for count of "g.V().has(\"weight\")"
```
--
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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487#issuecomment-1941684720
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2487?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`c7f3d24`)](https://app.codecov.io/gh/apache/tinkerpop/commit/c7f3d2454a654cea57c2a99077d859b115fec4c3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 75.15% compared to head [(`7c99643`)](https://app.codecov.io/gh/apache/tinkerpop/pull/2487?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 75.16%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.6-dev #2487 +/- ##
==========================================
Coverage 75.15% 75.16%
- Complexity 12351 12352 +1
==========================================
Files 1058 1058
Lines 63610 63610
Branches 6962 6962
==========================================
+ Hits 47806 47812 +6
+ Misses 13224 13217 -7
- Partials 2580 2581 +1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/2487?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487#issuecomment-1942116756
LGTM 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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette merged PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487
--
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
Re: [PR] TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #2487:
URL: https://github.com/apache/tinkerpop/pull/2487#discussion_r1488293645
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature:
##########
@@ -841,3 +841,45 @@ Feature: Step - mergeE()
"""
When iterated to list
Then the traversal will raise an error with message containing text of "Property key can not be a hidden key: ~label"
+
+ Scenario: g_V_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX
+ Given the empty graph
+ And the graph initializer of
+ """
+ g.addV("person").property("name", "marko").as("a").
+ addV("person").property("name", "vadas").as("b").
+ addE("knows").property("weight", 1).from("a").to("b")
+ """
+ And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\", \"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+ And the traversal of
+ """
+ g.V().mergeE(xx1).
+ option(Merge.onMatch, __.sideEffect(__.property("weight", 0)).constant([:]))
+ """
+ When iterated to list
+ Then the result should have a count of 2
+ And the graph should return 2 for count of "g.V()"
+ And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"weight\",1)"
+ And the graph should return 1 for count of "g.E().hasLabel(\"knows\").has(\"weight\",0)"
+ And the graph should return 0 for count of "g.V().hasLabel(\"knows\").has(\"weight\")"
Review Comment:
bad cut/paste - fixed both - thanks.
--
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