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)