Deprecate `AggregateLocalStep` and `aggregate(Scope, String)` in 3.7-dev #3234

xiazcy · 2025-10-07T16:15:42Z

Added deprecation for the removal of Scope in aggregate() in 3.8-dev, which is a follow-up to the Lazy vs Eager evaluation proposal. Duplicated existing aggregate(local, 'x') and store() tests with local(aggregate('x')) forms to ensure they are semantically equivalent.

… features tests for the preferred usage of `local(aggregate(String))`.

kenhuuu · 2025-10-09T21:50:04Z

CHANGELOG.asciidoc

 === TinkerPop 3.7.5 (Release Date: NOT OFFICIALLY RELEASED YET)

 * Improved Java driver host availability on connection pool initialization.
+* Deprecate `AggregateLocalStep` and `aggregate(Scope, String)`.


Suggested change

* Deprecate `AggregateLocalStep` and `aggregate(Scope, String)`.

* Deprecated `AggregateLocalStep` and `aggregate(Scope, String)`.

kenhuuu · 2025-10-09T21:51:07Z

docs/src/upgrade/release-3.7.x.asciidoc

+[source,text]
+----
+// 3.7.4
+gremlin> g.V().aggregate(Scope.local, "x").by("age").cap("x")


Not sure if this is the best example, as cap() only produces the last output so regardless of eager or lazy evaluation, the result should be the same. Maybe use select() instead instead cap().

kenhuuu · 2025-10-09T21:54:03Z

...-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Store.feature

+    Given the modern graph
+    And the traversal of
+      """
+      g.V().local(aggregate("a").by("name")).out().cap("a")


Same coment before about cap() vs select().

andreachild · 2025-10-10T16:26:59Z

docs/src/upgrade/release-3.7.x.asciidoc


+==== Deprecation of Scopes in aggregate() Step
+
+Scopes are now deprecated inside the `aggregate()` step, and will be removed in the next major upgrade.


Suggested change

Scopes are now deprecated inside the `aggregate()` step, and will be removed in the next major upgrade.

The acceptance of a scope parameter is now deprecated for the `aggregate()` step, and will be removed in the next major upgrade.

The suggestion has the right idea, but it should be "The acceptance of a Scope parameter to aggregate() step is now deprecated, and will be removed in the next major upgrade.

I think it would be good to further include some tie in to store which is already deprecated:

This deprecation, in combination with the deprecation of store in x.x.x, will help users begin to consider the implications of their usage of these steps as it pertains to lazy versus eager flow control by converting to more explicit Gremlin patterns that have more consistent and intuitive semantics.

That sentence can help lead into more details of those patterns and reasons "why" which @andreachild asked for.

andreachild · 2025-10-10T16:27:20Z

docs/src/upgrade/release-3.7.x.asciidoc

+==== Deprecation of Scopes in aggregate() Step
+
+Scopes are now deprecated inside the `aggregate()` step, and will be removed in the next major upgrade.
+The preferred way to achieve lazy evaluation with `aggregate()`, is using `local()`.


Please explain why this was done? Can reference the proposal if that helps.

I think it fine to reference the proposal, but it is dense and no one will read it. A good summary explanation as to the "why", with examples that clarify, is necessary imo.

andreachild · 2025-10-10T16:29:54Z

docs/src/upgrade/release-3.7.x.asciidoc

+// 3.7.4
+gremlin> g.V().aggregate(Scope.local, "x").by("age").cap("x")
+==>[29,27,32,35]
+
+// 3.7.5 convention
+gremlin> g.V().local(aggregate("x").by("age")).cap("x")
+==>[29,27,32,35]


Suggested change

// 3.7.4

gremlin> g.V().aggregate(Scope.local, "x").by("age").cap("x")

==>[29,27,32,35]

// 3.7.5 convention

gremlin> g.V().local(aggregate("x").by("age")).cap("x")

==>[29,27,32,35]

// 3.7.5 deprecated

gremlin> g.V().aggregate(Scope.local, "x").by("age").cap("x")

==>[29,27,32,35]

// 3.7.5 recommended

gremlin> g.V().local(aggregate("x").by("age")).cap("x")

==>[29,27,32,35]

andreachild · 2025-10-10T16:31:33Z

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java


    /**
     * @see GraphTraversal#aggregate(Scope, String)
+     * @deprecated As of release 3.7.5, aggregate() is no longer scoped, to achieve lazy evaluation use local() with aggregate() instead


Suggested rewording, same for all similar comments

Suggested change

* @deprecated As of release 3.7.5, aggregate() is no longer scoped, to achieve lazy evaluation use local() with aggregate() instead

* @deprecated As of release 3.7.5, aggregate() with scope parameter is deprecated - to achieve lazy evaluation use local() with aggregate() instead

i think you could go even shorter because the deprecation comment clearly applies to the Scope argument.

@deprecated As of release 3.7.5, prefer a pattern with {@link local()} and {@link aggregate()} instead

obviously, those links need fixup, but that's the idea.

andreachild · 2025-10-10T16:33:38Z

gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js

  // An associative array containing the scenario name as key, for example:
  'g_withSideEffectXa_setX_V_both_name_storeXaX_capXaX': new IgnoreError(ignoreReason.setNotSupported),
  'g_withSideEffectXa_setX_V_both_name_aggregateXlocal_aX_capXaX': new IgnoreError(ignoreReason.setNotSupported),
+  'g_withSideEffectXa_setX_V_both_name_localXaggregateX_aXX_capXaX': new IgnoreError(ignoreReason.setNotSupported),


Can you please add a comment as to why this is not supported?

andreachild · 2025-10-10T16:34:59Z

...-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Store.feature

+    And using the parameter vid1 defined as "v[marko].id"
+    And the traversal of
+      """
+      g.V(vid1).store("a").by("name").out().local(aggregate("a").by("name")).values("name").cap("a")


Should new tests for store be avoided since it is deprecated?

andreachild · 2025-10-10T16:37:55Z

...-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Store.feature

+    Given the modern graph
+    And the traversal of
+      """
+      g.V().local(aggregate("a").by("name")).out().cap("a")


Curious why this was added to the Store.feature if it doesn't use the store step?

andreachild · 2025-10-10T16:40:21Z

...e/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java

     * @since 3.4.3
+     * @deprecated As of release 3.7.5, aggregate() is no longer scoped, to achieve lazy evaluation use aggregate() inside of local()
     */
    public default GraphTraversal<S, E> aggregate(final Scope scope, final String sideEffectKey) {


Suggested change

public default GraphTraversal<S, E> aggregate(final Scope scope, final String sideEffectKey) {

@Deprecated

public default GraphTraversal<S, E> aggregate(final Scope scope, final String sideEffectKey) {

andreachild · 2025-10-10T16:40:51Z

...e/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java

Is a similar change needed to GraphTraversal.cs?

Cole-Greer · 2025-10-15T18:53:34Z

VOTE +1 pending resolution of above comments

spmallette · 2025-10-17T14:25:52Z

docs/src/upgrade/release-3.7.x.asciidoc


 === Upgrading for Users

+==== Deprecation of Scopes in aggregate() Step


"Scopes" really isn't the right term here. Also, the sections can't be too long or they wrap terribly in the ToC. How about just "Deprecate aggregate(Scope)`?

spmallette · 2025-10-17T14:51:00Z

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/__.java

     * @see GraphTraversal#aggregate(Scope, String)
+     * @deprecated As of release 3.7.5, aggregate() is no longer scoped, to achieve lazy evaluation use local() with aggregate() instead
     */
    public static <A> GraphTraversal<A, A> aggregate(final Scope scope, final String sideEffectKey) {


need a @Deprecated annotation?

spmallette · 2025-10-17T14:52:17Z

...e/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java

     * @return the traversal with an appended {@link AggregateGlobalStep}
     * @see <a href="http://tinkerpop.apache.org/docs/${project.version}/reference/#aggregate-step" target="_blank">Reference Documentation - Aggregate Step</a>
     * @since 3.4.3
+     * @deprecated As of release 3.7.5, aggregate() is no longer scoped, to achieve lazy evaluation use aggregate() inside of local()


adjust deprecation comment as noted with the __ class.

spmallette · 2025-10-17T14:55:35Z

.../java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AggregateLocalStep.java


 /**
 * @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @deprecated As of release 3.7.5, use local() with aggregate() instead


a bit of a nit i gues, but I think in this case, the deprecation comment is:

@deprecated As of release 3.7.5, not replaced

As a Gremlin step implementation, it's just gone...there's no direct replacement for usage. GraphTraversal and __ as user facing get the slightly fancier message.

xiazcy · 2025-10-27T23:08:34Z

Closing this PR, as we are no longer deprecating the steps in 3.7 due to local semantics changes (#3246) that will result in slightly altered behavior, and will make the removal in 3.8 a clean break.

Deprecated AggregateLocalStep and aggregate(Scope, String). Added…

8ebde8b

… features tests for the preferred usage of `local(aggregate(String))`.

kenhuuu reviewed Oct 9, 2025

View reviewed changes

andreachild reviewed Oct 10, 2025

View reviewed changes

spmallette reviewed Oct 17, 2025

View reviewed changes

spmallette mentioned this pull request Oct 17, 2025

Removed AggregateLocalStep, aggregate(Scope, String), and store() #3233

Merged

xiazcy closed this Oct 27, 2025

	* Deprecate `AggregateLocalStep` and `aggregate(Scope, String)`.
	* Deprecated `AggregateLocalStep` and `aggregate(Scope, String)`.


		==== Deprecation of Scopes in aggregate() Step

		Scopes are now deprecated inside the `aggregate()` step, and will be removed in the next major upgrade.

	Scopes are now deprecated inside the `aggregate()` step, and will be removed in the next major upgrade.
	The acceptance of a scope parameter is now deprecated for the `aggregate()` step, and will be removed in the next major upgrade.

	* @deprecated As of release 3.7.5, aggregate() is no longer scoped, to achieve lazy evaluation use local() with aggregate() instead
	* @deprecated As of release 3.7.5, aggregate() with scope parameter is deprecated - to achieve lazy evaluation use local() with aggregate() instead

	public default GraphTraversal<S, E> aggregate(final Scope scope, final String sideEffectKey) {
	@Deprecated
	public default GraphTraversal<S, E> aggregate(final Scope scope, final String sideEffectKey) {


		=== Upgrading for Users

		==== Deprecation of Scopes in aggregate() Step

Deprecate AggregateLocalStep and aggregate(Scope, String) in 3.7-dev #3234

Deprecate AggregateLocalStep and aggregate(Scope, String) in 3.7-dev #3234

Uh oh!

Conversation

xiazcy commented Oct 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreachild Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreachild Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cole-Greer commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiazcy commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Deprecate `AggregateLocalStep` and `aggregate(Scope, String)` in 3.7-dev #3234

Deprecate `AggregateLocalStep` and `aggregate(Scope, String)` in 3.7-dev #3234

andreachild Oct 10, 2025 •

edited

Loading

andreachild Oct 10, 2025 •

edited

Loading

xiazcy commented Oct 27, 2025 •

edited

Loading