This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch TINKERPOP-3056 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 7c9964381344279bda2dd82086bc4422ab7f613b Author: Stephen Mallette <stepm...@amazon.com> AuthorDate: Tue Feb 13 09:24:40 2024 -0500 TINKERPOP-3056 Fixed bug in mid-traversal mergeE for onMatch 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. --- CHANGELOG.asciidoc | 1 + .../process/traversal/step/map/MergeEdgeStep.java | 6 ++-- .../Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs | 2 ++ gremlin-go/driver/cucumber/gremlin.go | 2 ++ .../gremlin-javascript/test/cucumber/gremlin.js | 2 ++ gremlin-python/src/main/python/radish/gremlin.py | 2 ++ .../gremlin/test/features/map/MergeEdge.feature | 42 ++++++++++++++++++++++ 7 files changed, 54 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index e886f0a7ae..38ded69065 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,6 +32,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Fixed issue where server errors weren't being properly parsed when sending bytecode over HTTP. * Improved bulkset contains check for elements if all elements in bulkset are of the same type * Fixed bug in `EarlyLimitStrategy` which was too aggressive when promoting `limit()` before `map()`. +* Fixed bug in mid-traversal `mergeE()` where mutations in `sideEffect()` were being applied to the current traverser rather than a `onMatch` edge. [[release-3-6-6]] === TinkerPop 3.6.6 (November 20, 2023) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java index 748df266e0..761426efe3 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java @@ -273,9 +273,9 @@ public class MergeEdgeStep<S> extends MergeStep<S, Edge, Object> { edges = IteratorUtils.peek(edges, e -> { - // if this was a start step the traverser is initialized with placeholder edge, so override that with - // the matched Edge so that the option() traversal can operate on it properly - if (isStart) traverser.set((S) e); + // override current traverser with the matched Edge so that the option() traversal can operate + // on it properly + traverser.set((S) e); // assume good input from GraphTraversal - folks might drop in a T here even though it is immutable final Map<String, ?> onMatchMap = materializeMap(traverser, onMatchTraversal); diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs index 5fc05ee05b..9dc4aa5745 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs @@ -689,6 +689,8 @@ namespace Gremlin.Net.IntegrationTest.Gherkin {"g_mergeV_mergeE_combination_new_vertices", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.MergeV((IDictionary<object,object>) p["xx1"]).As("outV").MergeV((IDictionary<object,object>) p["xx2"]).As("inV").MergeE((IDictionary<object,object>) p["xx3"]).Option(Merge.OutV, (ITraversal) __.Select<object>("outV")).Option(Merge.InV, (ITraversal) __.Select<object>("inV")), (g,p) =>g.V(), (g,p) =>g.E(), (g,p) =>g.V().Has("name","marko").Out [...] {"g_mergeV_mergeE_combination_existing_vertices", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").AddV("person").Property("name","vadas"), (g,p) =>g.MergeV((IDictionary<object,object>) p["xx1"]).As("outV").MergeV((IDictionary<object,object>) p["xx2"]).As("inV").MergeE((IDictionary<object,object>) p["xx3"]).Option(Merge.OutV, (ITraversal) __.Select<object>("outV")).Option(Merge.InV, (ITraversal [...] {"g_V_asXvX_mergeEXxx1X_optionXMerge_onMatch_xx2X_optionXMerge_outV_selectXvXX_optionXMerge_inV_selectXvXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").Property("age",29), (g,p) =>g.V().As("v").MergeE((IDictionary<object,object>) p["xx1"]).Option(Merge.OnMatch, (IDictionary<object,object>) p["xx2"]).Option(Merge.OutV, (ITraversal) __.Select<object>("v")).Option(Merge.InV, (ITraversal) __. [...] + {"g_V_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").As("a").AddV("person").Property("name","vadas").As("b").AddE("knows").Property("weight",1).From("a").To("b"), (g,p) =>g.V().MergeE((IDictionary<object,object>) p["xx1"]).Option(Merge.OnMatch, (ITraversal) __.SideEffect(__.Property("weight [...] + {"g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").As("a").AddV("person").Property("name","vadas").As("b").AddE("knows").Property("weight",1).From("a").To("b"), (g,p) =>g.MergeE((IDictionary<object,object>) p["xx1"]).Option(Merge.OnMatch, (ITraversal) __.SideEffect(__.Property("weight",0)). [...] {"g_mergeVXemptyX_optionXonMatch_nullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").Property("age",29), (g,p) =>g.MergeV((IDictionary<object,object>) new Dictionary<object,object> {}).Option(Merge.OnMatch, (IDictionary<object,object>) null), (g,p) =>g.V().Has("person","name","marko").Has("age",29)}}, {"g_V_mergeVXemptyX_optionXonMatch_nullX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").Property("age",29), (g,p) =>g.V().MergeV((IDictionary<object,object>) new Dictionary<object,object> {}).Option(Merge.OnMatch, (IDictionary<object,object>) null), (g,p) =>g.V().Has("person","name","marko").Has("age",29)}}, {"g_mergeVXnullX_optionXonCreate_label_null_name_markoX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name","marko").Property("age",29), (g,p) =>g.MergeV((IDictionary<object,object>) p["xx1"])}}, diff --git a/gremlin-go/driver/cucumber/gremlin.go b/gremlin-go/driver/cucumber/gremlin.go index 801cafa8c5..8968aabb54 100644 --- a/gremlin-go/driver/cucumber/gremlin.go +++ b/gremlin-go/driver/cucumber/gremlin.go @@ -660,6 +660,8 @@ var translationMap = map[string][]func(g *gremlingo.GraphTraversalSource, p map[ "g_mergeV_mergeE_combination_new_vertices": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.MergeV(p["xx1"]).As("outV").MergeV(p["xx2"]).As("inV").MergeE(p["xx3"]).Option(gremlingo.Merge.OutV, gremlingo.T__.Select("outV")).Option(gremlingo.Merge.InV, gremlingo.T__.Select("inV"))}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V()}, func(g *gremlingo.GraphTraversalSource, p m [...] "g_mergeV_mergeE_combination_existing_vertices": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "marko").AddV("person").Property("name", "vadas")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.MergeV(p["xx1"]).As("outV").MergeV(p["xx2"]).As("inV").MergeE(p["xx3"]).Option(gremlingo.Merge.OutV, gremlingo.T__.Select("outV")).Option(gremlingo.Me [...] "g_V_asXvX_mergeEXxx1X_optionXMerge_onMatch_xx2X_optionXMerge_outV_selectXvXX_optionXMerge_inV_selectXvXX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "marko").Property("age", 29)}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().As("v").MergeE(p["xx1"]).Option(gremlingo.Merge.OnMatch, p["xx2"]).Option(gremlingo.Merge.OutV, gremlingo.T_ [...] + "g_V_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "marko").As("a").AddV("person").Property("name", "vadas").As("b").AddE("knows").Property("weight", 1).From("a").To("b")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().MergeE(p["xx1"]) [...] + "g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "marko").As("a").AddV("person").Property("name", "vadas").As("b").AddE("knows").Property("weight", 1).From("a").To("b")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.MergeE(p["xx1"]).Optio [...] "g_mergeVXemptyX_optionXonMatch_nullX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "marko").Property("age", 29)}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.MergeV(map[interface{}]interface{}{}).Option(gremlingo.Merge.OnMatch, nil)}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V [...] "g_V_mergeVXemptyX_optionXonMatch_nullX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "marko").Property("age", 29)}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().MergeV(map[interface{}]interface{}{}).Option(gremlingo.Merge.OnMatch, nil)}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {retu [...] "g_mergeVXnullX_optionXonCreate_label_null_name_markoX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "marko").Property("age", 29)}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.MergeV(p["xx1"])}}, diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js index 94e701723a..ce3f2a6809 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js @@ -679,6 +679,8 @@ const gremlins = { g_mergeV_mergeE_combination_new_vertices: [function({g, xx1, xx3, xx2}) { return g.mergeV(xx1).as("outV").mergeV(xx2).as("inV").mergeE(xx3).option(Merge.outV,__.select("outV")).option(Merge.inV,__.select("inV")) }, function({g, xx1, xx3, xx2}) { return g.V() }, function({g, xx1, xx3, xx2}) { return g.E() }, function({g, xx1, xx3, xx2}) { return g.V().has("name","marko").out("knows").has("name","vadas") }], g_mergeV_mergeE_combination_existing_vertices: [function({g, xx1, xx3, xx2}) { return g.addV("person").property("name","marko").addV("person").property("name","vadas") }, function({g, xx1, xx3, xx2}) { return g.mergeV(xx1).as("outV").mergeV(xx2).as("inV").mergeE(xx3).option(Merge.outV,__.select("outV")).option(Merge.inV,__.select("inV")) }, function({g, xx1, xx3, xx2}) { return g.V() }, function({g, xx1, xx3, xx2}) { return g.E() }, function({g, xx1, xx3, xx2}) { return g.V().has("na [...] g_V_asXvX_mergeEXxx1X_optionXMerge_onMatch_xx2X_optionXMerge_outV_selectXvXX_optionXMerge_inV_selectXvXX: [function({g, xx1, xx2}) { return g.addV("person").property("name","marko").property("age",29) }, function({g, xx1, xx2}) { return g.V().as("v").mergeE(xx1).option(Merge.onMatch,xx2).option(Merge.outV,__.select("v")).option(Merge.inV,__.select("v")) }], + g_V_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX: [function({g, xx1}) { return g.addV("person").property("name","marko").as("a").addV("person").property("name","vadas").as("b").addE("knows").property("weight",1).from_("a").to("b") }, function({g, xx1}) { return g.V().mergeE(xx1).option(Merge.onMatch,__.sideEffect(__.property("weight",0)).constant(new Map([]))) }, function({g, xx1}) { return g.V() }, function({g, xx1}) { return [...] + g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX: [function({g, xx1}) { return g.addV("person").property("name","marko").as("a").addV("person").property("name","vadas").as("b").addE("knows").property("weight",1).from_("a").to("b") }, function({g, xx1}) { return g.mergeE(xx1).option(Merge.onMatch,__.sideEffect(__.property("weight",0)).constant(new Map([]))) }, function({g, xx1}) { return g.V() }, function({g, xx1}) { return g.E(). [...] g_mergeVXemptyX_optionXonMatch_nullX: [function({g}) { return g.addV("person").property("name","marko").property("age",29) }, function({g}) { return g.mergeV(new Map([])).option(Merge.onMatch,null) }, function({g}) { return g.V().has("person","name","marko").has("age",29) }], g_V_mergeVXemptyX_optionXonMatch_nullX: [function({g}) { return g.addV("person").property("name","marko").property("age",29) }, function({g}) { return g.V().mergeV(new Map([])).option(Merge.onMatch,null) }, function({g}) { return g.V().has("person","name","marko").has("age",29) }], g_mergeVXnullX_optionXonCreate_label_null_name_markoX: [function({g, xx1}) { return g.addV("person").property("name","marko").property("age",29) }, function({g, xx1}) { return g.mergeV(xx1) }], diff --git a/gremlin-python/src/main/python/radish/gremlin.py b/gremlin-python/src/main/python/radish/gremlin.py index eebd6d69d2..dbb436fd49 100644 --- a/gremlin-python/src/main/python/radish/gremlin.py +++ b/gremlin-python/src/main/python/radish/gremlin.py @@ -661,6 +661,8 @@ world.gremlins = { 'g_mergeV_mergeE_combination_new_vertices': [(lambda g, xx1=None,xx3=None,xx2=None:g.merge_v(xx1).as_('outV').merge_v(xx2).as_('inV').merge_e(xx3).option(Merge.out_v,__.select('outV')).option(Merge.in_v,__.select('inV'))), (lambda g, xx1=None,xx3=None,xx2=None:g.V()), (lambda g, xx1=None,xx3=None,xx2=None:g.E()), (lambda g, xx1=None,xx3=None,xx2=None:g.V().has('name','marko').out('knows').has('name','vadas'))], 'g_mergeV_mergeE_combination_existing_vertices': [(lambda g, xx1=None,xx3=None,xx2=None:g.addV('person').property('name','marko').addV('person').property('name','vadas')), (lambda g, xx1=None,xx3=None,xx2=None:g.merge_v(xx1).as_('outV').merge_v(xx2).as_('inV').merge_e(xx3).option(Merge.out_v,__.select('outV')).option(Merge.in_v,__.select('inV'))), (lambda g, xx1=None,xx3=None,xx2=None:g.V()), (lambda g, xx1=None,xx3=None,xx2=None:g.E()), (lambda g, xx1=None,xx3=None,xx2=None:g.V().ha [...] 'g_V_asXvX_mergeEXxx1X_optionXMerge_onMatch_xx2X_optionXMerge_outV_selectXvXX_optionXMerge_inV_selectXvXX': [(lambda g, xx1=None,xx2=None:g.addV('person').property('name','marko').property('age',29)), (lambda g, xx1=None,xx2=None:g.V().as_('v').merge_e(xx1).option(Merge.on_match,xx2).option(Merge.out_v,__.select('v')).option(Merge.in_v,__.select('v')))], + 'g_V_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX': [(lambda g, xx1=None:g.addV('person').property('name','marko').as_('a').addV('person').property('name','vadas').as_('b').addE('knows').property('weight',1).from_('a').to('b')), (lambda g, xx1=None:g.V().merge_e(xx1).option(Merge.on_match,__.sideEffect(__.property('weight',0)).constant({}))), (lambda g, xx1=None:g.V()), (lambda g, xx1=None:g.E().hasLabel('knows').has('weight',1 [...] + 'g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_sideEffectXpropertyXweight_0XX_constantXemptyXX': [(lambda g, xx1=None:g.addV('person').property('name','marko').as_('a').addV('person').property('name','vadas').as_('b').addE('knows').property('weight',1).from_('a').to('b')), (lambda g, xx1=None:g.merge_e(xx1).option(Merge.on_match,__.sideEffect(__.property('weight',0)).constant({}))), (lambda g, xx1=None:g.V()), (lambda g, xx1=None:g.E().hasLabel('knows').has('weight',1)), (l [...] 'g_mergeVXemptyX_optionXonMatch_nullX': [(lambda g:g.addV('person').property('name','marko').property('age',29)), (lambda g:g.merge_v({}).option(Merge.on_match,None)), (lambda g:g.V().has('person','name','marko').has('age',29))], 'g_V_mergeVXemptyX_optionXonMatch_nullX': [(lambda g:g.addV('person').property('name','marko').property('age',29)), (lambda g:g.V().merge_v({}).option(Merge.on_match,None)), (lambda g:g.V().has('person','name','marko').has('age',29))], 'g_mergeVXnullX_optionXonCreate_label_null_name_markoX': [(lambda g, xx1=None:g.addV('person').property('name','marko').property('age',29)), (lambda g, xx1=None:g.merge_v(xx1))], diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature index ae30e00f9f..8e0663110b 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature +++ b/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\")" \ No newline at end of file