Skip to content

TINKERPOP-3244 Add NextN(n) to Traversal in gremlin-go#3416

Open
L0Lmaker wants to merge 3 commits intoapache:masterfrom
L0Lmaker:improvement/TINKERPOP-3244-go-next-n
Open

TINKERPOP-3244 Add NextN(n) to Traversal in gremlin-go#3416
L0Lmaker wants to merge 3 commits intoapache:masterfrom
L0Lmaker:improvement/TINKERPOP-3244-go-next-n

Conversation

@L0Lmaker
Copy link
Copy Markdown
Contributor

@L0Lmaker L0Lmaker commented May 2, 2026

Summary

  • Adds Traversal.NextN(n int) ([]*Result, error) to gremlin-go for batched result iteration, providing API parity with next(n) in the Java, Python, and .NET GLVs.
  • Behavior matches the Java reference (Traversal.java:107): returns up to n results, returns what is available if fewer exist, returns an empty slice for n <= 0, and surfaces ResultSet errors.
  • Method name is NextN rather than Next(n int) because Go does not support method overloading — a literal port of the Java signature would collide with the existing Next() (*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() vs ResultSet.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) vs 3.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 master for now. Please let me know your preference — happy to open companion PRs against 3.7-dev and 3.8-dev with implementations matching their respective iteration models if that is how you would like it landed.

Test plan

  • go build ./... clean
  • go vet ./driver/ clean
  • gofmt clean
  • 9 new unit tests in TestTraversalNextN cover: n < available, n == available, n > available, n = 0, n < 0, exhausted traversal, bulked traverser unrolling across batch boundary, repeated drain calls, error propagation
  • Existing tests (TestTraversalNextValue, TestTraversal/Test_clone_traversal, TestTraversal/Test_Iterate_with_empty_removeConnection) still pass
  • Full integration suite via ./run.sh 4.0.0-beta.2: 2017/2043 cucumber scenarios pass, all gremlin-go tests including TestTraversalNextN pass. 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 published 4.0.0-beta.2 server image.

Notes

This change was AI-assisted; the commit includes the Assisted-by: trailer per AGENTS.md.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.24%. Comparing base (cfd6889) to head (584ada5).
⚠️ Report is 1035 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@spmallette
Copy link
Copy Markdown
Contributor

spmallette commented May 2, 2026

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:

Assisted-by: Claude:claude-opus-4-7 [Claude Code]

It signed like this too for me yesterday as a first time using that instruction. I'd envisioned:

Assisted-by: Claude Code:claude-opus-4-7

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
@L0Lmaker L0Lmaker force-pushed the improvement/TINKERPOP-3244-go-next-n branch from 0169556 to d396815 Compare May 2, 2026 16:00
@L0Lmaker
Copy link
Copy Markdown
Contributor Author

L0Lmaker commented May 2, 2026

Thanks @spmallette! Updated the commit message — the trailer now reads Assisted-by: Claude Code:claude-opus-4-7. Branch force-pushed at d396815e4c.

For future reference (and anyone else interpreting the AGENTS.md template), I read AGENT_NAME:MODEL_VERSION [TOOL] literally and matched the existing precedent in 0531cbb7d0 (Assisted-by: Kiro:claude-opus-4.6 [kiro-cli]), which appears to have hit the same ambiguity. Happy to look at AGENTS.md wording in a separate PR if helpful.

@Cole-Greer
Copy link
Copy Markdown
Contributor

Thanks for opening the PR, the NextN(n) syntax seems like a good workaround considering Go's lack of overloading support. Since this differs from the other drivers, I think it might warrant a brief shoutout in the Go differences reference docs.

This should also get added to the Go translators in Java and JS.

L0Lmaker added 2 commits May 2, 2026 22:34
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
@L0Lmaker
Copy link
Copy Markdown
Contributor Author

L0Lmaker commented May 3, 2026

Thanks for the review @Cole-Greer. Pushed 584ada5d4d to address both points:

  • Go translators in gremlin-core and gremlin-javascript now emit NextN(n) for the next(n) terminal step (no-arg next() continues to map to Next()). Implemented as a visitTraversalTerminalMethod_next override in each translator, since the symbol-mapping layer doesn't see the integer-argument distinction. Existing parameterized test rows updated.
  • Added a short paragraph in gremlin-variants.asciidoc under the Go === Differences section explaining why the batched form is exposed as a sibling method.
  • Expanded the CHANGELOG bullet to also name the translator updates, matching the prior gremlin-go + GoTranslatorVisitor entry already in the 4.0.0 section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants