Allow Steps to Accept Traversal

[xiazcy]

Summary

Allow child traversals as arguments to has(), is(), V(), E(), property(), where(P), and all P/TextP predicates. The child traversal is evaluated per-traverser at runtime, enabling dynamic value resolution.

JIRA:

• TINKERPOP-2586: Traversal objects inside within() silently return empty instead of executing or erroring - fully addressed (traversals are now executed)
• TINKERPOP-2777: V() cannot accept traversals like select() for data-driven lookups - fully addressed
• TINKERPOP-3005: property() should accept a traversal producing a Map - fully addressed
• TINKERPOP-1463: has(key, traversal) doesn't work as expected - partially addressed (dynamic filtering works; runtime folding into index lookups is deferred as a future optimization)

What it enables

// with Modern graph
// Data-driven vertex lookup (TINKERPOP-2777)
gremlin> g.inject(['1','2','3']).as('a').V(__.select('a')).values('name')
==>marko
==>vadas
==>lop

// Dynamic filtering - find people older than marko
gremlin> g.V().has('age', P.gt(__.V(1).values('age'))).values('name')
==>josh
==>peter

// Computed property mutation (TINKERPOP-3005) - set multiple properties from a Map
gremlin> g.V(4).property(__.V(1).project('friendCount','createdSoftware').by(__.out('knows').count()).by(__.out('created').values('name')))
==>v[4]
gremlin> g.V(4).valueMap('friendCount','createdSoftware')
==>[friendCount:[2],createdSoftware:[lop]]

// with AirRoutes graph
// Find airports where the city name ends with the airport code (lowercased)
gremlin> g.V().has("city", TextP.endingWith(__.values("code").toLower())).limit(10).values("code","city").fold()
==>[CUN, Cancun, ORK, Cork, OME, Nome, IEV, Kiev, AJU, Aracaju, IRA, Kirakira, OHE, Mohe, RUA, Arua, VAS, Sivas, AZD, Yazd]

---

Review Guide

This is a large change (68 files, ~7k lines) but structured in layers. Review bottom-up:

#	Commit	Layer	What to look for
1	5f37d11	Predicate resolution	Core: P.resolve(), empty-result semantics, AndP short-circuit, ConnectiveP/NotP delegation
2	6a547d2	Step implementations	How steps use P.resolve(). HasStep single path, GraphStep idTraversal, AddPropertyStep mapForm, AcceptsChildPredicateTraversal marker
3	e80423a	Grammar	Gremlin.g4 nestedTraversal alternatives. Visitors delegate to API (not addStep())
4	1b21d15	GLVs	within()/without() serialization. .NET typed overloads
5	793e7a8	Tests + docs	Cucumber .feature files define behavioral contract. Reference doc updates
6	400f5ae	Cleanup	Dead code removal from HasContainer unification. Test quality fixes

Architecture (single resolution path)

                    has("age", __.V(1).values("age"))
                              │
                    DSL wraps: has("age", P.eq(traversal))
                              │
                              ▼
                    HasContainer(key, P.eq(traversal))
                              │
              ┌───────────────┴───────────────┐
              │ HasStep.resolvePredicate()     │
              │   P.resolve(traverser)         │
              │   hc.test(element)             │
              └────────────────────────────────┘

Key decisions to scrutinize

1. Single resolution path - At the DSL level, has(key, traversal) wraps the traversal in P.eq(traversal) before creating the HasContainer. This means HasStep only has one code path for traversal resolution: call P.resolve(traverser) on the predicate, then hc.test(element). The GremlinLang serialization still records the original has(key, traversal) form (verified by round-trip tests). This is a new 4.0.0 feature, so no existing provider code is affected. Providers implementing custom strategies must check HasContainer.hasTraversal() before folding into index lookups.

2. Empty-result semantics - When a child traversal produces zero results, behavior depends on predicate type. Collection predicates (within/without) resolve to an empty collection and delegate to Contains.test() which applies correct set-theoretic logic: within([]) is always false, without([]) is always true. Scalar predicates (eq/gt/lt/etc.) set resolvedEmpty=true and steps short-circuit to false since there is no meaningful comparison value. A null result (traversal produces null) is distinct from an empty result: it resolves normally and null is used as the comparison value, consistent with existing P.eq(null) semantics.

3. AndP short-circuits, OrP doesn't - AndP.resolve() overrides ConnectiveP.resolve() to stop resolving children at the first scalar predicate that resolves empty. This is sound because and(unsatisfiable, X) is always false regardless of X. OrP does NOT short-circuit: a non-empty resolution does not guarantee the predicate will pass test(), so later children must already be resolved when their turn comes. Note: short-circuiting means side-effecting child traversals (e.g., ones using aggregate) in later AndP operands may not execute. This matches standard boolean short-circuit semantics.

4. P.resolve() mutates in place - resolve() overwrites literals, isCollection, and resolvedEmpty fields on the P instance. In OLTP this is safe since resolve and test happen sequentially per traverser. In OLAP, HasStep.clone() deep-clones predicates per worker, providing thread isolation. The mutation pattern is a known limitation documented for future improvement (return an immutable resolved value instead). Predicate trees of arbitrary depth (AndP containing OrP containing NotP, etc.) are handled by recursive resolution and recursive cloning.

5. Mutation guard - ChildTraversalValidator rejects any child traversal containing a step that implements the Mutating interface (addV, addE, drop, property). This runs at construction time (DSL methods and P factory methods) and again at strategy time via ChildTraversalVerificationStrategy. Aggregating side-effects (aggregate, store, group) are NOT blocked because they don't modify graph state. Whether they should be restricted is a question for reviewers.

6. Provider contract - HasContainer.hasTraversal() is the single check point providers must use before folding. Providers that fold HasContainers into index-backed steps without this check will attempt to use an unresolved P value (returns null before resolve() is called) as an index key, producing wrong results. TinkerGraphStepStrategy demonstrates the pattern: skip traversal-bearing HasSteps with continue so that subsequent literal HasSteps can still be folded. Providers with custom GraphStep replacements must also copy the idTraversal field during strategy replacement, otherwise V(traversal) falls through to returning all vertices.

7. choose() restriction - choose(P) and choose().option(P, ...) reject traversal-bearing predicates at construction time. This is a deliberate scope decision. PredicateTraversal (which evaluates option-key predicates in BranchStep) has access to the traverser and could technically call P.resolve() before P.test(), but extending the branching infrastructure is out of scope for this feature. Users needing dynamic branching can use the choose(Traversal, ...) form where the branch predicate is a full traversal (e.g., choose(__.is(P.gt(traversal)), trueChoice, falseChoice)). This works because IsStep handles resolution normally. Restricting is() when used inside choose()'s branch traversal is not feasible since choose() receives it as an opaque Traversal.Admin and does not inspect internal steps.

What's NOT in this PR

• P.resolve() thread-safety refactoring (future)
• instanceof DefaultGraphTraversal cleanup (deferred)
• Runtime index folding for deterministic child traversals
• FilterRankingStrategy cost awareness

Compatibility

• Additive overloads only. No existing step or predicate signature changed.
• Serialized GremlinLang form is unchanged: has(key, traversal) still serializes as has(key, traversal), verified by GremlinLangTraversalRoundTripTest and GremlinQueryParserTraversalTest.

---

Testing

All gremlin-core unit tests pass (9132, 0 failures) and the full TinkerGraph Gherkin suite passes (2172 scenarios, 0 failures) against the current master.

Category	Coverage
Behavioral contract	HasTraversal.feature, IsTraversal.feature, WhereTraversal.feature, VETraversal.feature, PropertyTraversal.feature, ChildTraversalVerification.feature
Internal mechanics	PTraversalTest, CloneIndependenceTraversalTest, HasContainerTest, ChildTraversalValidatorTest, ChildTraversalVerificationStrategyTest
Grammar + serialization	GremlinQueryParserTraversalTest, GremlinLangTraversalRoundTripTest
Strategy correctness	TinkerGraphStepStrategyTraversalTest

Checklist

• CHANGELOG entry added
• Reference documentation updated (the-traversal.asciidoc)
• Upgrade documentation added (release-4.x.x.asciidoc)
• All GLVs updated (Python, .NET, Go, JavaScript)
• Full mvn clean install passes
• No new external dependencies introduced

---

VOTE +1

[spmallette]

The child traversal must be read-only - mutating steps are rejected.

Don't really like dashes in our docs this way, how about: "The child traversal cannot include mutating steps like addV() or mergeE(). If these steps are present, they will be rejected at runtime with a XYZException."

[spmallette]

any reason to not just fold this back into the previous examples? i'm not sure it needs to stand out as a separate thing.

[spmallette]

again, don't like the dash this way in our docs:

The child traversal must be read-only - mutating steps are rejected.

you could use the other verbiage i had or some variation of it.

If the traversal does not produce a Map, the result is rejected.

rejected when and in with what exception?

[spmallette]

please format the query to make this easier to read.

[spmallette]

maybe better to show the whole vertex with all properties to demonstrate we augmented an existing vertex that had properteis?

[spmallette]

if you get a better multi-line formatting as i mentioned above, then this description can be broken into smaller bits for folks to understand more easily

[spmallette]

again, i feel like this doesn't need to be an "also" in how this step works. just integrate it into the other examples and content more seamlessly.

[spmallette]

again, as with other comments, let's integrate this more seamlessly into the content. Also, perhaps where is a case where we could come up with a more complex example since it's a more advanced step.

[spmallette]

Need a catchier title here. "Expanding Dynamic Arguments"??

[spmallette]

Folks have been waiting on this feature for a long time. It needs a strong introduction with some history about the challenges folks had with the old way to do some of this stuff. Show some comparisons demonstrating the old way versus the ease of the new way.

Stuff like the following is instructional details and doesn't really belong here:

Child traversals take only the first result (consistent with by(traversal) semantics). For within()/without(),
use fold() to collect multiple values into a list.

i think we should also be telling users in this section what patterns this feature allows them to replace to make their code perform better, read better, etc.

[spmallette]

Aren't there some JIRAs for this to reference?

[spmallette]

There are some docs here that cover the feature, but what I don't see are updates to existing docs that use old patterns that are no longer relevant and that we should no longer promote. Has that kind of search been done?

[spmallette]

All of these *Traversal.feature tests should be integrated to their parent step files.

For this file, the fact that every single test has @GraphComputerVerificationMidVNotSupported tells me something is wrong because it either means we've mis-labeled the test (as happened elsewhere) or we don't have all the right testing.

Where are the tests for use cases with select() for example? I know V() is popular but select() will equally come into play. That approach and others seem necessary. What about filtering on a sack() value? that might be interesting.

[Cole-Greer]

I've migrated all of these *Traversal.feature to their appropriate parent step file. For has() specifically, I've removed excessive @GraphComputerVerificationMidVNotSupported tags and added a new case for select() and sack()

[spmallette]

same general comments as for has() - https://github.com/apache/tinkerpop/pull/3458/changes#r3443937188

[spmallette]

same general comments as for has() - https://github.com/apache/tinkerpop/pull/3458/changes#r3443937188

[spmallette]

I don't believe the multi-traversal semantics are correct. In the reference docs we describe it as:

• The multi-traversal form within(trav1, trav2, ...) takes the first result from each traversal and combines them into a collection for membership testing.

but within doesn't really work like that:

gremlin> g.V().has('name',within(['josh','lop'], 'vadas'))
==>v[2]

It only expands the list if the first and singular argument is a collection:

gremlin> g.V().has('name',within(['vadas','josh','lop']))
==>v[2]
==>v[3]
==>v[4]

Otherwise, as shown in the first example, it will treat it like comparing on a List and a String. V and E have similar semantics, but oddly:

gremlin> g.V().hasId(2, [3, 1])
==>v[2]
==>v[3]
==>v[1]
gremlin> g.V(2, [3, 1])
Expected an id that is convertible to class java.lang.Integer but received class java.util.ArrayList - [[3, 1]]
Type ':help' or ':h' for help.
Display stack trace? [yN]

This goes back to https://issues.apache.org/jira/browse/TINKERPOP-2863 where I think there is a bug in the implementation which tried to get hasId to be like V and E but allowed a little too much to happen. I think all of this needs to be rectified.

[Cole-Greer]

I've updated the "auto-unfolding" semantics to properly align with the literal-form of within()

gremlin> g.inject(1).is(P.within(constant([1,2]), constant(3)))
gremlin> g.inject(1).is(P.within(constant([1,2,3])))
==>1

[spmallette]

Given my comment here; https://github.com/apache/tinkerpop/pull/3458/changes#r3443997744 i think this should be written as:

g.V().has("name", P.within(__.union(__.V(vid1).out("knows").values("name"), 
                                    __.V(vid3).out("created").values("name")).fold()))

[spmallette]

The Gremlin in our tests should be formatted to match our best practices. These tests need to continue to be a solid dependable source for human curated Gremlin.

[Cole-Greer]

@spmallette Regarding your comment here:

There are some docs here that cover the feature, but what I don't see are updates to existing docs that use old patterns that are no longer relevant and that we should no longer promote. Has that kind of search been done?

I got an agent to do a scan through all of our documentation for examples in need of updates, I found a few cases in the recipes docs, notable in anti-patterns and traversal-induced-values. The anti pattern has been removed entirely, and the traversal-induced-values example has been updated to the new recommended pattern.

I haven't done any manual reads through the bulk of the docs in regards to this feature alone, but I intend to do a thorough docs review in the lead up to the 4.0.0-beta.3 release which may reveal some other examples which would benefit from refinement.

[spmallette]

overall the test organization looks better since i called in out in my first review, but i'm still not seeing where we've expanded tests much to cover cases that don't use mid-V.

[spmallette]

additional reference link for V(Traversal)?

[spmallette]

have we examined if we are over-iterating here given how strategies tend to be applied plus the recursive call in ReadOnlyChildValidator? perhaps there is a room for some added efficiency here?

[Cole-Greer]

Yes, and we excessively "double up" in one case, if a ReadOnlyTraversalParent is itself a descendant of another ReadOnlyTraversalParent, the child's children will be needlessly scanned for Mutating steps a second time.

I've accepted that as a reasonable trade off for simplicity as I expect that to be a fairly rare case given the intended usage of these steps. If we were to avoid recursing into the descendants of a ReadOnlyTraversalParent and instead rely on the recursive application of the strategy to all children, we would instead need to include a test if traversal itself is a descendant of a ReadOnlyTraversalParent which we don't have a great way to do, and would likely negate any savings from double recursion.

[Cole-Greer]

Taking another pass over this code, I think it can be simplified a bit by relying on TraversalHelper.getStepsOfAssignableClass(ReadOnlyTraversalParent.class). This wouldn't really impact the performance much but would be a decent simplification of the code.

[spmallette]

I'm not completely sure it's innocuous as that, but irrespective of that point I'm now wondering why this is a separate strategy. Why wouldn't we fold this functionality under the StandardVerificationStrategy? This is very standard behavior that must be executed - I can't think of a reason you would opt-out of just this verification. Folding the feature into "standard" would allow the validation to happen within the same iteration of steps as the existing verification instead of a second pass. It would also remove a bunch of GLV code and unnecessary testing.

[Cole-Greer]

We should recurse through global children as well. I don't think any of the current set of ReadOnlyTraversalParent steps actually have any global children so there's no impact right now, but it would be more future proof.

Suggested change

	for (final Traversal.Admin<?, ?> child : ((ReadOnlyTraversalParent) step).getLocalChildren()) {
	try {
	ReadOnlyChildValidator.validate(child);
	} catch (final IllegalArgumentException e) {
	throw new VerificationException(e.getMessage(), traversal);
	}
	}
	for (final Traversal.Admin<?, ?> child : ((ReadOnlyTraversalParent) step).getLocalChildren()) {
	try {
	ReadOnlyChildValidator.validate(child);
	} catch (final IllegalArgumentException e) {
	throw new VerificationException(e.getMessage(), traversal);
	}
	}
	for (final Traversal.Admin<?, ?> child : ((ReadOnlyTraversalParent) step).getGlobalChildren()) {
	try {
	ReadOnlyChildValidator.validate(child);
	} catch (final IllegalArgumentException e) {
	throw new VerificationException(e.getMessage(), traversal);
	}
	}