You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by "stephen mallette (JIRA)" <ji...@apache.org> on 2019/06/07 19:34:00 UTC
[jira] [Updated] (TINKERPOP-1553) Deprecate store() in favor of
aggregate(Scope)
[ 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)