This is an automated email from the ASF dual-hosted git repository. xiazcy pushed a commit to branch TINKERPOP-3273 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 1112fe8a30114499e86b6cf8ca2fb95df0129cad Author: Yang Xia <[email protected]> AuthorDate: Wed Jul 22 10:01:06 2026 -0700 fix: Align hasId() collection unrolling with V()/E() hasId() previously unrolled a Collection into individual ids even when it was passed alongside other arguments, so g.V().hasId(1, [2,4]) matched v[1], v[2] and v[4]. This was inconsistent with g.V()/g.E(), which only unroll a single Collection argument and otherwise treat each argument as a literal id. hasId() now unrolls a collection or array only when it is the sole argument. With multiple arguments, each is treated as a literal id, so a start-step hasId() folds into the graph step and behaves exactly like the equivalent V()/E() lookup. A single collection argument continues to unroll as before (TINKERPOP-2863). TINKERPOP-3273 Assisted-by: Kiro:claude-opus-4.8 --- CHANGELOG.asciidoc | 1 + docs/src/upgrade/release-3.7.x.asciidoc | 32 ++++++++++++++++++++++ .../traversal/dsl/graph/GraphTraversal.java | 31 ++++++++------------- .../process/traversal/step/filter/HasStepTest.java | 3 +- .../Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs | 1 + gremlin-go/driver/cucumber/gremlin.go | 1 + .../gremlin-javascript/test/cucumber/gremlin.js | 1 + .../src/main/python/tests/feature/gremlin.py | 1 + .../gremlin/test/features/filter/HasId.feature | 16 ++++++++++- .../tinkergraph/structure/TinkerGraphTest.java | 32 ++++++++++++++++++++++ 10 files changed, 96 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 5e54133fcc..0e7c26017d 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -29,6 +29,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Added `next(n)` to `Traversal` in `gremlin-javascript` for batched result iteration, providing API parity with `next(n)` in the Java, Python, and .NET GLVs. * Added `WithComputer()` to `GraphTraversalSource` in `gremlin-go`, providing OLAP configuration parity with other language variants. * Added `FailResponseException` to `gremlin-driver` which is thrown `fail()` step is triggered on the server making it more consistent with embedded behavior. +* Fixed `hasId()` to only unroll a collection when it is supplied as the single argument, aligning its behavior with `g.V()`/`g.E()`. * Fixed conjoin has incorrect null handling. * Expanded `gremlin-python` CI matrix to test against Python 3.9, 3.10, 3.11, 3.12, and 3.13. * Add Node 26 support for `gremlin-javascript` and `gremlint`. diff --git a/docs/src/upgrade/release-3.7.x.asciidoc b/docs/src/upgrade/release-3.7.x.asciidoc index 0d7e175746..89fc92891b 100644 --- a/docs/src/upgrade/release-3.7.x.asciidoc +++ b/docs/src/upgrade/release-3.7.x.asciidoc @@ -76,6 +76,38 @@ Note that this change is for Java only and designed to better align embedded and See: link:https://issues.apache.org/jira/browse/TINKERPOP-3238[TINKERPOP-3238] +==== hasId() Collection Unrolling + +`hasId()` now only unrolls a collection into individual identifiers when that collection is supplied as the *single* +argument to the step. This aligns its behavior with `g.V()` and `g.E()`, where only a single `Collection` of ids is +unrolled. Previously, `hasId()` would unroll a collection even when it was mixed with other arguments, which was +inconsistent with `g.V()`/`g.E()`. + +[source,groovy] +---- +// 3.7.6 - the collection is unrolled even alongside other ids +gremlin> g.V().hasId(1, [2, 4]) +==>v[1] +==>v[2] +==>v[4] + +// 3.7.7 - a collection mixed with other arguments is treated as a literal id (not unrolled), +// consistent with g.V(1, [2, 4]) +gremlin> g.V().hasId(1, [2, 4]) +// no longer matches v[2] and v[4] by unrolling the list + +// unchanged: a single collection argument is still unrolled +gremlin> g.V().hasId([1, 2]) +==>v[1] +==>v[2] +---- + +Code that relied on `hasId()` unrolling a collection that was passed alongside other id arguments should be updated to +either pass all ids as a single collection (for example `g.V().hasId([1, 2, 4])`) or as individual arguments (for +example `g.V().hasId(1, 2, 4)`). + +See: link:https://issues.apache.org/jira/browse/TINKERPOP-3273[TINKERPOP-3273] + == TinkerPop 3.7.6 *Release Date: April 1, 2026* diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java index fedafea38e..32a2408bce 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java @@ -2390,28 +2390,19 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { else { this.asAdmin().getBytecode().addStep(Symbols.hasId, id, otherIds); - //using ArrayList given P.within() turns all arguments into lists + // a collection/array is only unrolled when it is the single argument, aligning hasId() with g.V()/g.E() + // where multiple arguments are each treated as a literal id (TINKERPOP-3273) final List<Object> ids = new ArrayList<>(); - if (id instanceof Object[]) { - Collections.addAll(ids, (Object[]) id); - } else if (id instanceof Collection) { - // as ids are unrolled when it's in array, they should also be unrolled when it's a list. - // this also aligns with behavior of hasId() when it's pushed down to g.V() (TINKERPOP-2863) - ids.addAll((Collection<?>) id); - } else + if (otherIds == null || otherIds.length == 0) { + if (id instanceof Object[]) { + Collections.addAll(ids, (Object[]) id); + } else if (id instanceof Collection) { + ids.addAll((Collection<?>) id); + } else + ids.add(id); + } else { ids.add(id); - - // unrolling ids from lists works cleaner with Collection too, as otherwise they will need to - // be turned into array first - if (otherIds != null) { - for (final Object i : otherIds) { - if (id instanceof Object[]) { - Collections.addAll(ids, (Object[]) i); - } else if (i instanceof Collection) { - ids.addAll((Collection<?>) i); - } else - ids.add(i); - } + Collections.addAll(ids, otherIds); } return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.id.getAccessor(), ids.size() == 1 ? P.eq(ids.get(0)) : P.within(ids))); diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStepTest.java index ae7114b769..dfc21056b7 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStepTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStepTest.java @@ -73,8 +73,7 @@ public class HasStepTest extends StepTest { __.hasId(1, 2), __.hasId(within(1, 2)), __.hasId(new Integer[]{1, 2}), - __.hasId(Arrays.asList(1, 2)), - __.hasId(Collections.singletonList(1), Collections.singletonList(2))), + __.hasId(Arrays.asList(1, 2))), // hasLabel(Object label, Object... moreLabels) should be compatible with hasLabel(Object... labels) Arrays.asList( diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs index 0a8e187dc2..baf112b027 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs @@ -291,6 +291,7 @@ namespace Gremlin.Net.IntegrationTest.Gherkin {"g_VX1X_out_hasXid_2_3X_inList", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V(p["vid1"]).Out().HasId(p["xx1"])}}, {"g_V_hasXid_1_2X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasId(p["vid1"],p["vid2"])}}, {"g_V_hasXid_1_2X_inList", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasId(p["xx1"])}}, + {"g_VX1X_out_hasIdX2_listXid3_id4XX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V(p["vid1"]).Out().HasId(p["vid2"],p["xx1"])}}, {"g_V_both_dedup_properties_hasKeyXageX_value", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Both().Properties<object>().Dedup().HasKey("age").Value<object>()}}, {"g_V_both_properties_dedup_hasKeyXageX_hasValueXgtX30XX_value", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Both().Properties<object>().Dedup().HasKey("age").HasValue(P.Gt(30)).Value<object>(), (g,p) =>g.V().Both().Properties<object>().Dedup().HasKey("age").HasValue(P.Gt(30)).Value<object>()}}, {"g_V_bothE_properties_dedup_hasKeyXweightX_value", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().BothE().Properties<object>().Dedup().HasKey("weight").Value<object>()}}, diff --git a/gremlin-go/driver/cucumber/gremlin.go b/gremlin-go/driver/cucumber/gremlin.go index ba1a6b1ae0..4d284cb8e9 100644 --- a/gremlin-go/driver/cucumber/gremlin.go +++ b/gremlin-go/driver/cucumber/gremlin.go @@ -262,6 +262,7 @@ var translationMap = map[string][]func(g *gremlingo.GraphTraversalSource, p map[ "g_VX1X_out_hasXid_2_3X_inList": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V(p["vid1"]).Out().HasId(p["xx1"])}}, "g_V_hasXid_1_2X": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().HasId(p["vid1"], p["vid2"])}}, "g_V_hasXid_1_2X_inList": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().HasId(p["xx1"])}}, + "g_VX1X_out_hasIdX2_listXid3_id4XX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V(p["vid1"]).Out().HasId(p["vid2"], p["xx1"])}}, "g_V_both_dedup_properties_hasKeyXageX_value": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Both().Properties().Dedup().HasKey("age").Value()}}, "g_V_both_properties_dedup_hasKeyXageX_hasValueXgtX30XX_value": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Both().Properties().Dedup().HasKey("age").HasValue(gremlingo.P.Gt(30)).Value()}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Both().Properties().Dedup().HasKey("age").HasValue(gremlingo.P.Gt(30)).Value()}}, "g_V_bothE_properties_dedup_hasKeyXweightX_value": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().BothE().Properties().Dedup().HasKey("weight").Value()}}, 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 b6de78479b..b49f992762 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 @@ -282,6 +282,7 @@ const gremlins = { g_VX1X_out_hasXid_2_3X_inList: [function({g, xx1, vid1}) { return g.V(vid1).out().hasId(xx1) }], g_V_hasXid_1_2X: [function({g, vid2, vid1}) { return g.V().hasId(vid1,vid2) }], g_V_hasXid_1_2X_inList: [function({g, xx1}) { return g.V().hasId(xx1) }], + g_VX1X_out_hasIdX2_listXid3_id4XX: [function({g, xx1, vid2, vid1}) { return g.V(vid1).out().hasId(vid2,xx1) }], g_V_both_dedup_properties_hasKeyXageX_value: [function({g}) { return g.V().both().properties().dedup().hasKey("age").value() }], g_V_both_properties_dedup_hasKeyXageX_hasValueXgtX30XX_value: [function({g}) { return g.V().both().properties().dedup().hasKey("age").hasValue(P.gt(30)).value() }, function({g}) { return g.V().both().properties().dedup().hasKey("age").hasValue(P.gt(30)).value() }], g_V_bothE_properties_dedup_hasKeyXweightX_value: [function({g}) { return g.V().bothE().properties().dedup().hasKey("weight").value() }], diff --git a/gremlin-python/src/main/python/tests/feature/gremlin.py b/gremlin-python/src/main/python/tests/feature/gremlin.py index 3d14a7224d..22e51e7b4b 100644 --- a/gremlin-python/src/main/python/tests/feature/gremlin.py +++ b/gremlin-python/src/main/python/tests/feature/gremlin.py @@ -264,6 +264,7 @@ world.gremlins = { 'g_VX1X_out_hasXid_2_3X_inList': [(lambda g, xx1=None,vid1=None:g.V(vid1).out().has_id(xx1))], 'g_V_hasXid_1_2X': [(lambda g, vid2=None,vid1=None:g.V().has_id(vid1,vid2))], 'g_V_hasXid_1_2X_inList': [(lambda g, xx1=None:g.V().has_id(xx1))], + 'g_VX1X_out_hasIdX2_listXid3_id4XX': [(lambda g, xx1=None,vid2=None,vid1=None:g.V(vid1).out().has_id(vid2,xx1))], 'g_V_both_dedup_properties_hasKeyXageX_value': [(lambda g:g.V().both().properties().dedup().has_key('age').value())], 'g_V_both_properties_dedup_hasKeyXageX_hasValueXgtX30XX_value': [(lambda g:g.V().both().properties().dedup().has_key('age').has_value(P.gt(30)).value()), (lambda g:g.V().both().properties().dedup().has_key('age').has_value(P.gt(30)).value())], 'g_V_bothE_properties_dedup_hasKeyXweightX_value': [(lambda g:g.V().both_e().properties().dedup().has_key('weight').value())], diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/HasId.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/HasId.feature index 02e145a3ee..434dd37438 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/HasId.feature +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/HasId.feature @@ -253,4 +253,18 @@ Feature: Step - hasId() Then the result should be unordered | result | | v[marko] | - | v[vadas] | \ No newline at end of file + | v[vadas] | + + Scenario: g_VX1X_out_hasIdX2_listXid3_id4XX + Given the modern graph + And using the parameter vid1 defined as "v[marko].id" + And using the parameter vid2 defined as "v[vadas].id" + And using the parameter xx1 defined as "l[v[lop].id,v[josh].id]" + And the traversal of + """ + g.V(vid1).out().hasId(vid2, xx1) + """ + When iterated to list + Then the result should be unordered + | result | + | v[vadas] | diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java index 8d4ab9e739..9e0ee4fee7 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java @@ -69,6 +69,7 @@ import java.io.FileOutputStream; import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -87,6 +88,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeThat; @@ -706,6 +708,36 @@ public class TinkerGraphTest { assertEquals(expectedMidTraversal, g.V().has("name", "marko").outE("knows").inV().hasId(Arrays.asList(2, 4)).toList()); } + /** + * Validating that hasId() only unrolls a collection when it is supplied as the single argument, matching the + * behavior of g.V()/g.E(). When multiple arguments are supplied, a collection argument is treated as a literal + * id rather than being unrolled (TINKERPOP-3273). + */ + @Test + public void shouldNotUnrollCollectionWhenMidTraversalHasIdHasMultipleArguments() { + final GraphTraversalSource g = TinkerFactory.createModern().traversal(); + + final List<Vertex> expected = g.V().has("name", "marko").outE("knows").inV().hasId(2).toList(); + assertEquals(expected, g.V().has("name", "marko").outE("knows").inV().hasId(2, Collections.singletonList(4)).toList()); + } + + /** + * Validating that a start-step hasId() with multiple arguments where one is a collection is consistent with + * g.V()/g.E(): the collection is not unrolled and the graph rejects it as a non-convertible id (TINKERPOP-3273). + */ + @Test + public void shouldBeConsistentWithVWhenStartStepHasIdHasMultipleArguments() { + final GraphTraversalSource g = TinkerFactory.createModern().traversal(); + + final IllegalArgumentException vException = assertThrows(IllegalArgumentException.class, + () -> g.V(1, Arrays.asList(2, 4)).toList()); + assertThat(vException.getMessage(), containsString("Expected an id that is convertible to")); + + final IllegalArgumentException hasIdException = assertThrows(IllegalArgumentException.class, + () -> g.V().hasId(1, Arrays.asList(2, 4)).toList()); + assertThat(hasIdException.getMessage(), containsString("Expected an id that is convertible to")); + } + @Test public void shouldOptionalUsingWithComputer() { // not all systems will have 3+ available processors (e.g. travis)
