[
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)
