This is an automated email from the ASF dual-hosted git repository. xiazcy pushed a commit to branch remove-choose-option-traversal in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit c15d2573e96d30882ca22f8e56223a5700c8dd28 Author: xiazcy <[email protected]> AuthorDate: Thu Oct 16 20:02:00 2025 -0700 disallow the use of choose().option(Traversal, v) --- CHANGELOG.asciidoc | 1 + docs/src/dev/provider/gremlin-semantics.asciidoc | 5 ++- docs/src/upgrade/release-3.8.x.asciidoc | 39 +++++++++++++++++++++- .../process/traversal/step/branch/ChooseStep.java | 6 ++-- .../traversal/step/branch/ChooseStepTest.java | 10 ++++++ .../Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs | 1 + gremlin-go/driver/cucumber/gremlin.go | 1 + .../gremlin-javascript/test/cucumber/gremlin.js | 1 + gremlin-python/src/main/python/radish/gremlin.py | 1 + .../gremlin/test/features/branch/Choose.feature | 13 +++++++- 10 files changed, 71 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 04ae491b5c..77dd2147ad 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -69,6 +69,7 @@ This release also includes changes from <<release-3-7-XXX, 3.7.XXX>>. * Changed `choose` to only use the first `option` matched. * Added `Pick.unproductive` to allow for matches on unproductive predicates. * Changed `choose` to pass through traversers of unproductive predicates and unmatched choices. +* Disallowed `Traversal` as a `Pick` token for `option` in `choose`. * Updated `OptionsStrategy` in `gremlin-python` to take options directly as keyword arguments. * Added static `instance()` method to `ElementIdStrategy` to an instance with the default configuration. * Updated `ElementIdStrategy.getConfiguration()` to help with serialization. diff --git a/docs/src/dev/provider/gremlin-semantics.asciidoc b/docs/src/dev/provider/gremlin-semantics.asciidoc index dff6cda769..c83ec68d2f 100644 --- a/docs/src/dev/provider/gremlin-semantics.asciidoc +++ b/docs/src/dev/provider/gremlin-semantics.asciidoc @@ -1138,9 +1138,8 @@ link:https://tinkerpop.apache.org/docs/x.y.z/reference/#call-step[reference] *Modulation:* * `option(pickToken, traversalOption)` - Adds a traversal option to the `choose` step. The `pickToken` is matched -against the result of the choice traversal. The `pickToken` may be a literal value, a predicate `P`, a `Traversal` -(whose first returned value is used for matching) or a `Pick` enum value. If a match is found, the traverser is routed -to the corresponding `traversalOption`. +against the result of the choice traversal. The `pickToken` may be a literal value, a predicate `P` or a `Pick` enum +value. `Traversal` is not allowed as a `pickToken` here and will lead to `IllegalArgumentException`. If a match is found, the traverser is routed to the corresponding `traversalOption`. *Considerations:* diff --git a/docs/src/upgrade/release-3.8.x.asciidoc b/docs/src/upgrade/release-3.8.x.asciidoc index d404c2b915..8c5530edf3 100644 --- a/docs/src/upgrade/release-3.8.x.asciidoc +++ b/docs/src/upgrade/release-3.8.x.asciidoc @@ -623,8 +623,45 @@ gremlin> g.V().choose(__.values("age")). ==>peter ---- +*Removal of choose().option(Traversal, v)* + +The `choose().option(Traversal, v)` is rarely used in comparison to the other overloads with constants, predicates and Pick +tokens. It is also not the recommended approach for performance reasons. In addition, the current implementation leads to +confusion as it only evaluates if the traversal is productive, rather than performing comparison on based on the traversal's +output value. Fixing this behavior would require extensive changes, as such we are removing this functionality from +`choose()` in 3.8.0, with plans to reintroduce a properly implemented version in the next major release. + +[source,text] +---- +// 3.7.x +gremlin> g.V().hasLabel("person").choose(values("name")). +......1> option(is("marko"), values("age")). +......2> option(none, values("name")) +==>29 +==>vadas +==>josh +==>peter + +// 3.8.0 - an IllegalArgumentException will be thrown +gremlin> g.V().hasLabel("person").choose(values("name")). +......1> option(is("marko"), values("age")). +......2> option(none, values("name")) +Traversal is not allowed as a Pick token for choose().option() +Type ':help' or ':h' for help. +Display stack trace? [yN]n +// use predicates in these cases +gremlin> g.V().hasLabel("person").choose(values("name")). +......1> option(P.eq("marko"), values("age")). +......2> option(none, values("name")) +==>29 +==>vadas +==>josh +==>peter +---- + See: link:https://issues.apache.org/jira/browse/TINKERPOP-3178[TINKERPOP-3178], -link:https://tinkerpop.apache.org/docs/3.8.0/reference/#choose-step[Reference Documentation - choose()] +link:https://tinkerpop.apache.org/docs/3.8.0/reference/#choose-step[Reference Documentation - choose()], +linke:https://lists.apache.org/thread/mtfy1jshb8rwqglp7ooxswwwwj70qy33[DISCUSS] ==== Float Defaults to Double diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java index 9ca0d101ec..366ad6bfde 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStep.java @@ -62,11 +62,11 @@ public final class ChooseStep<S, E, M> extends BranchStep<S, E, M> { private boolean hasDefaultNone = true; private ChooseStep(final Traversal.Admin traversal, final Traversal.Admin<S, M> choiceTraversal, - final ChooseSemantics chooseSemantics) { + final ChooseSemantics chooseSemantics) { super(traversal); this.chooseSemantics = chooseSemantics; this.setBranchTraversal(choiceTraversal); - + // defaults Pick.unproductive/none options are just identity() final Traversal.Admin<S,E> unproductivePassthrough = new DefaultGraphTraversal<>(); unproductivePassthrough.addStep(new IdentityStep<>(traversal)); @@ -120,6 +120,8 @@ public final class ChooseStep<S, E, M> extends BranchStep<S, E, M> { } else if (pickToken == Pick.none && !this.traversalPickOptions.containsKey(Pick.none)) { super.addChildOption(pickToken, traversalOption); } + } else if (pickToken instanceof Traversal) { + throw new IllegalArgumentException("Traversal is not allowed as a Pick token for choose().option()"); } else { super.addChildOption(pickToken, traversalOption); } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStepTest.java index 604ae20ec8..ac7218b3f5 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStepTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/ChooseStepTest.java @@ -22,6 +22,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest; +import org.junit.Test; import java.util.Arrays; import java.util.List; @@ -30,6 +31,7 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.in; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.values; import static org.apache.tinkerpop.gremlin.process.traversal.Pick.none; +import static org.junit.Assert.assertThrows; /** * @author Daniel Kuppitz (http://gremlin.guru) @@ -48,4 +50,12 @@ public class ChooseStepTest extends StepTest { __.choose(out("knows").is(P.gt(0)), out("knows")) ); } + + @Test + public void shouldRejectTraversalAsOptionToken() { + assertThrows(IllegalArgumentException.class, () -> { + __.choose(__.hasLabel("person")). + option(__.values("name"), __.out("knows")); + }); + } } diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs index 9ab8812476..0b0afbe3a5 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs @@ -104,6 +104,7 @@ namespace Gremlin.Net.IntegrationTest.Gherkin {"g_V_chooseXhasXname_vadasX__valuesXnameXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Choose<object>(__.Has("name", "vadas"), __.Values<object>("name"))}}, {"g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX_constantXotherXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person").Values<object>("age").Choose<object>(P.Eq(29), __.Constant<object>("matched"), __.Constant<object>("other"))}}, {"g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person").Values<object>("age").Choose<object>(P.Eq(29), __.Constant<object>("matched"))}}, + {"g_V_hasLabelXpersonX_chooseX_valuesXnameX_option1X_isXmarkoX_valuesXageXX_option2Xnone_valuesXnameXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person").Choose<object>(__.Values<object>("name")).Option(__.Is("marko"), __.Values<object>("age")).Option(Pick.None, __.Values<object>("name"))}}, {"g_V_localXpropertiesXlocationX_order_byXvalueX_limitX2XX_value", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Local<object>(__.Properties<object>("location").Order().By(T.Value, Order.Asc).Range<object>(0, 2)).Value<object>()}}, {"g_V_hasXlabel_personX_asXaX_localXoutXcreatedX_asXbXX_selectXa_bX_byXnameX_byXidX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Has(T.Label, "person").As("a").Local<object>(__.Out("created").As("b")).Select<object>("a", "b").By("name").By(T.Id)}}, {"g_V_localXoutE_countX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Local<object>(__.OutE().Count())}}, diff --git a/gremlin-go/driver/cucumber/gremlin.go b/gremlin-go/driver/cucumber/gremlin.go index 3c3d92fc6b..66601987d5 100644 --- a/gremlin-go/driver/cucumber/gremlin.go +++ b/gremlin-go/driver/cucumber/gremlin.go @@ -74,6 +74,7 @@ var translationMap = map[string][]func(g *gremlingo.GraphTraversalSource, p map[ "g_V_chooseXhasXname_vadasX__valuesXnameXX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Choose(gremlingo.T__.Has("name", "vadas"), gremlingo.T__.Values("name"))}}, "g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX_constantXotherXX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().HasLabel("person").Values("age").Choose(gremlingo.P.Eq(29), gremlingo.T__.Constant("matched"), gremlingo.T__.Constant("other"))}}, "g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().HasLabel("person").Values("age").Choose(gremlingo.P.Eq(29), gremlingo.T__.Constant("matched"))}}, + "g_V_hasLabelXpersonX_chooseX_valuesXnameX_option1X_isXmarkoX_valuesXageXX_option2Xnone_valuesXnameXX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().HasLabel("person").Choose(gremlingo.T__.Values("name")).Option(gremlingo.T__.Is("marko"), gremlingo.T__.Values("age")).Option(gremlingo.Pick.None, gremlingo.T__.Values("name"))}}, "g_V_localXpropertiesXlocationX_order_byXvalueX_limitX2XX_value": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Local(gremlingo.T__.Properties("location").Order().By(gremlingo.T.Value, gremlingo.Order.Asc).Range(0, 2)).Value()}}, "g_V_hasXlabel_personX_asXaX_localXoutXcreatedX_asXbXX_selectXa_bX_byXnameX_byXidX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Has(gremlingo.T.Label, "person").As("a").Local(gremlingo.T__.Out("created").As("b")).Select("a", "b").By("name").By(gremlingo.T.Id)}}, "g_V_localXoutE_countX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Local(gremlingo.T__.OutE().Count())}}, 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 e01d05fbc1..4eb77fb07e 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 @@ -105,6 +105,7 @@ const gremlins = { g_V_chooseXhasXname_vadasX__valuesXnameXX: [function({g}) { return g.V().choose(__.has("name", "vadas"), __.values("name")) }], g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX_constantXotherXX: [function({g}) { return g.V().hasLabel("person").values("age").choose(P.eq(29), __.constant("matched"), __.constant("other")) }], g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX: [function({g}) { return g.V().hasLabel("person").values("age").choose(P.eq(29), __.constant("matched")) }], + g_V_hasLabelXpersonX_chooseX_valuesXnameX_option1X_isXmarkoX_valuesXageXX_option2Xnone_valuesXnameXX: [function({g}) { return g.V().hasLabel("person").choose(__.values("name")).option(__.is("marko"), __.values("age")).option(Pick.none, __.values("name")) }], g_V_localXpropertiesXlocationX_order_byXvalueX_limitX2XX_value: [function({g}) { return g.V().local(__.properties("location").order().by(T.value, Order.asc).range(0, 2)).value() }], g_V_hasXlabel_personX_asXaX_localXoutXcreatedX_asXbXX_selectXa_bX_byXnameX_byXidX: [function({g}) { return g.V().has(T.label, "person").as("a").local(__.out("created").as("b")).select("a", "b").by("name").by(T.id) }], g_V_localXoutE_countX: [function({g}) { return g.V().local(__.outE().count()) }], diff --git a/gremlin-python/src/main/python/radish/gremlin.py b/gremlin-python/src/main/python/radish/gremlin.py index 3fd2da2666..d314544281 100644 --- a/gremlin-python/src/main/python/radish/gremlin.py +++ b/gremlin-python/src/main/python/radish/gremlin.py @@ -77,6 +77,7 @@ world.gremlins = { 'g_V_chooseXhasXname_vadasX__valuesXnameXX': [(lambda g:g.V().choose(__.has('name', 'vadas'), __.values('name')))], 'g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX_constantXotherXX': [(lambda g:g.V().has_label('person').values('age').choose(P.eq(29), __.constant('matched'), __.constant('other')))], 'g_V_hasLabelXpersonX_age_chooseXP_eqX29X_constantXmatchedX': [(lambda g:g.V().has_label('person').values('age').choose(P.eq(29), __.constant('matched')))], + 'g_V_hasLabelXpersonX_chooseX_valuesXnameX_option1X_isXmarkoX_valuesXageXX_option2Xnone_valuesXnameXX': [(lambda g:g.V().has_label('person').choose(__.values('name')).option(__.is_('marko'), __.values('age')).option(Pick.none, __.values('name')))], 'g_V_localXpropertiesXlocationX_order_byXvalueX_limitX2XX_value': [(lambda g:g.V().local(__.properties('location').order().by(T.value, Order.asc).range_(0, 2)).value())], 'g_V_hasXlabel_personX_asXaX_localXoutXcreatedX_asXbXX_selectXa_bX_byXnameX_byXidX': [(lambda g:g.V().has(T.label, 'person').as_('a').local(__.out('created').as_('b')).select('a', 'b').by('name').by(T.id_))], 'g_V_localXoutE_countX': [(lambda g:g.V().local(__.out_e().count()))], diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/branch/Choose.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/branch/Choose.feature index f6c24ff724..37f4a1b4ae 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/branch/Choose.feature +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/branch/Choose.feature @@ -666,4 +666,15 @@ Feature: Step - choose() | matched | | d[27].i | | d[32].i | - | d[35].i | \ No newline at end of file + | d[35].i | + + Scenario: g_V_hasLabelXpersonX_chooseX_valuesXnameX_option1X_isXmarkoX_valuesXageXX_option2Xnone_valuesXnameXX + Given the modern graph + And the traversal of + """ + g.V().hasLabel("person").choose(values("name")). + option(is("marko"), values("age")). + option(Pick.none, values("name")) + """ + When iterated to list + Then the traversal will raise an error with message containing text of "Traversal is not allowed as a Pick token"
