[ https://issues.apache.org/jira/browse/TINKERPOP-1553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
stephen mallette updated TINKERPOP-1553: ---------------------------------------- Labels: deprecation (was: breaking) Summary: Deprecate store() in favor of aggregate(Scope) (was: AggregateStep should go away in favor of StoreStep.NoOpBarrierStep) I've always liked the idea of this change, as having {{store()}} and {{aggregate()}} provided the same feature with just different semantics for doing so. we've held off on doing this for a while because it was a breaking change. i was able to remove that "breaking" label though by preferring to keep {{aggregate()}} rather than {{store()}} - by keeping {{aggregate()}} we end up with the "global" version of the step as the default. in this way we can deprecate {{store()}} and redirect folks to {{aggregate(Scope,String)}} using {{Scope.local}}. So, that makes this one a bit easier to swallow from the API perspective. On a separate note, the by-product of this change was that we thought we might lose some code in that {{AggregateStep}} could go away in favor of {{store().barrier()}} combination, but there are some reasons why this is more of a change to the Gremlin language than it is to reducing code redundancy. Taken literally, simply adding a {{NoOpBarrierStep}} after {{StoreStep}} in {{GraphTraversal.store()}} if the {{Scope}} is {{global}} creates an issue if there is a {{by()}} modulator following the {{store()}} because it will end up modulating the {{NoOpBarrierStep}} rather than the {{StoreStep}}. In addition there are certain semantics around {{by()}} and {{aggregate()}} / {{store()}} that I don't think we want to see change - see this example: {code} gremlin> g.V().hasLabel('software').store('x').cap('x').unfold() ==>v[3] ==>v[5] gremlin> g.V().hasLabel('software').aggregate('x').cap('x').unfold() ==>v[3] ==>v[5] gremlin> g.V().hasLabel('software').store('x').by(union(identity(), select('x').count(local)).fold()).cap('x').unfold() ==>[v[3],0] ==>[v[5],1] gremlin> g.V().hasLabel('software').aggregate('x').by(union(identity(), select('x').count(local)).fold()).cap('x').unfold() ==>[v[3],0] ==>[v[5],0] {code} So, that said, we probably need to keep the existing logic in both {{AggregateStep}} and {{StoreStep}}. I think we can just deprecate both in favor of a new {{AggregateGlobalStep}} and {{AggregateLocalStep}} respectively which would match patterns on other scoped steps like {{max()}} and {{min()}}. > Deprecate store() in favor of aggregate(Scope) > ---------------------------------------------- > > Key: TINKERPOP-1553 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1553 > Project: TinkerPop > Issue Type: Improvement > Components: process > Affects Versions: 3.2.3 > Reporter: Marko A. Rodriguez > Assignee: stephen mallette > Priority: Major > Labels: deprecation > > `AggregateStep` can be expressed as `StoreStep.NoOpBarrierStep`. There is no > reason to have the extra logic if we don't need it. > That is: > {code} > aggregate('a') => store('a').barrier() > {code} > Next, we should get rid of {{aggregate()}} and have two methods: > {code} > store(global,'a') => store('a').barrier() > store(local,'a') => store('a') > {code} > If you are storing global that means you are storing every traverser up to > the current step. Likewise, store local would only store the current > traverser. > Here is the crappy thing. All of our {{xxx(Scope)}} steps default to > {{Scope.global}}: {{range()}}, {{tail()}}, {{count()}}... > We should probably keep the same pattern of {{Scope.global}} default, but > then that means that we would have a breaking change in the API. > {code} > store("a") -would-change-to-> store(local,"a") > {code} > We should have a {{storeV3d0()}} backward compatibility which would simply > use {{store(local,"a")}}. -- This message was sent by Atlassian JIRA (v7.6.3#76005)