Cole-Greer commented on code in PR #3055:
URL: https://github.com/apache/tinkerpop/pull/3055#discussion_r1964484965
##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature:
##########
@@ -947,4 +947,45 @@ Feature: Step - mergeE()
And the graph should return 7 for count of "g.E()"
And the graph should return 7 for count of "g.E().hasNot(\"created\")"
And the graph should return 1 for count of
"g.V().has(\"person\",\"name\",\"marko\").outE(\"knows\").hasNot(\"created\").inV().has(\"person\",\"name\",\"vadas\")"
- And the graph should return 1 for count of
"g.V().has(\"person\",\"name\",\"vadas\").outE(\"self\").hasNot('weight').inV().has(\"person\",\"name\",\"vadas\")"
\ No newline at end of file
+ And the graph should return 1 for count of
"g.V().has(\"person\",\"name\",\"vadas\").outE(\"self\").hasNot('weight').inV().has(\"person\",\"name\",\"vadas\")"
+
+ @AllowNullPropertyValues
+ Scenario:
g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_weight_nullX_allowed
+ 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").from("a").to("b").property("weight",1.0d)
+ """
+ And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\",
\"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+ And using the parameter xx2 defined as "m[{\"weight\":null}]"
+ And the traversal of
+ """
+ g.mergeE(xx1).option(Merge.onMatch,xx2)
+ """
+ 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 1 for count of "g.E().hasLabel(\"knows\")"
+ And the graph should return 1 for count of
"g.E().hasLabel(\"knows\").has(\"weight\",null)"
+
+ Scenario:
g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_weight_nullX
+ 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").from("a").to("b").property("weight",1.0d)
+ """
+ And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\",
\"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+ And using the parameter xx2 defined as "m[{\"weight\":null}]"
+ And the traversal of
+ """
+ g.mergeE(xx1).option(Merge.onMatch,xx2)
+ """
+ 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 1 for count of "g.E().hasLabel(\"knows\")"
+ And the graph should return 0 for count of
"g.E().hasLabel(\"knows\").has(\"weight\")"
Review Comment:
It feels wrong to me to have scenarios which will fail on a graph with null
properties enabled and cannot easily be singled out and skipped. I guess I
don't need to turn this PR into a referendum on this issue though especially
considering the precedent has already been set. I'm ok if you proceed with
merging this PR either as-is, or with a new tag to skip such scenarios.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
