TINKERPOP-3244 Add NextN(n) to Traversal in gremlin-go#3416
TINKERPOP-3244 Add NextN(n) to Traversal in gremlin-go#3416L0Lmaker wants to merge 3 commits intoapache:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3416 +/- ##
============================================
- Coverage 77.87% 76.24% -1.63%
+ Complexity 13578 13317 -261
============================================
Files 1015 1011 -4
Lines 59308 59962 +654
Branches 6835 7011 +176
============================================
- Hits 46184 45717 -467
- Misses 10817 11547 +730
- Partials 2307 2698 +391 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for the PR. I'll leave it to the Go experts to dig into the details here but I'm interested in the signoff since I recently added that instruction to our AGENTS setup:
It signed like this too for me yesterday as a first time using that instruction. I'd envisioned:
I might need to better define that to get what I want there. It's interpreting the "tool" to be Claude Code - maybe because the tool is not seen as optional?? @L0Lmaker would you mind modifying that commit message so that agents don't pick up a structure we didn't intend. |
Adds a batched-iteration terminator that returns up to n results, mirroring next(n) in the Java, Python, and .NET GLVs. Go does not support method overloading, so this lands as a sibling method to the existing Next() rather than a second Next(int) signature. Behavior matches the Java reference: returns up to n results, returns what is available if fewer exist, returns an empty slice for non-positive n, and surfaces ResultSet errors to the caller. Assisted-by: Claude Code:claude-opus-4-7
0169556 to
d396815
Compare
|
Thanks @spmallette! Updated the commit message — the trailer now reads For future reference (and anyone else interpreting the AGENTS.md template), I read |
|
Thanks for opening the PR, the This should also get added to the Go translators in Java and JS. |
Addresses review feedback on PR apache#3416: - gremlin-core and gremlin-js Go translators now emit NextN(n) for the batched form of the next() terminal step instead of the (invalid) Next(n). next() with no argument continues to translate to Next(). - Adds a note in the Go variants reference docs explaining that the batched form is exposed as a distinct method because Go does not support method overloading. Test expectations in GremlinTranslatorTest and gremlin-translator-test.js updated to match. Assisted-by: Claude Code:claude-opus-4-7
Expands the existing 4.0.0 bullet to also name the gremlin-core and gremlin-javascript translator updates, matching the pattern used by the prior gremlin-go + GoTranslatorVisitor entry on the same release. Assisted-by: Claude Code:claude-opus-4-7
|
Thanks for the review @Cole-Greer. Pushed
|
Summary
Traversal.NextN(n int) ([]*Result, error)to gremlin-go for batched result iteration, providing API parity withnext(n)in the Java, Python, and .NET GLVs.Traversal.java:107): returns up tonresults, returns what is available if fewer exist, returns an empty slice forn <= 0, and surfacesResultSeterrors.NextNrather thanNext(n int)because Go does not support method overloading — a literal port of the Java signature would collide with the existingNext() (*Result, error). This matches the existing pattern in gremlin-go where overloaded Java APIs are split into distinctly-named Go methods (e.g.ResultSet.One()vsResultSet.All()).JIRA
TINKERPOP-3244
Backport question for maintainers
The JIRA lists Affects Versions
3.7.5, 3.8.1. Per the PR template I would normally target the earliest branch and let it merge forward, but the iteration model in gremlin-go diverged on master (GremlinLang+nextValue()helper) vs3.7-dev/3.8-dev(Bytecode+One()directly). The implementation here is master-shaped and won't forward-merge cleanly without a rewrite for the older protocol.Targeting
masterfor now. Please let me know your preference — happy to open companion PRs against3.7-devand3.8-devwith implementations matching their respective iteration models if that is how you would like it landed.Test plan
go build ./...cleango vet ./driver/cleangofmtcleanTestTraversalNextNcover:n < available,n == available,n > available,n = 0,n < 0, exhausted traversal, bulked traverser unrolling across batch boundary, repeated drain calls, error propagationTestTraversalNextValue,TestTraversal/Test_clone_traversal,TestTraversal/Test_Iterate_with_empty_removeConnection) still pass./run.sh 4.0.0-beta.2: 2017/2043 cucumber scenarios pass, all gremlin-go tests includingTestTraversalNextNpass. The single failing scenario (g_injectXnull_nullX_conjoinXplusX) is unrelated to this change — pre-existing client/server version skew where the master client expects the post-TINKERPOP-3225 server fix that is not in the published4.0.0-beta.2server image.Notes
This change was AI-assisted; the commit includes the
Assisted-by:trailer perAGENTS.md.