[
https://issues.apache.org/jira/browse/TINKERPOP-2396?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17176083#comment-17176083
]
redtree commented on TINKERPOP-2396:
------------------------------------
Ok I think the idea that a provider should have some strategy to swap the
TraverserSet if necessary does make sense to me.
To do so, I think we still need my initial implementation + AbstractStep and
ExpandableStepIterator should have some method to replace the logic to spawn a
new TraverserSet.
So in my latest pull request, I added *AbstractStep#setTraverserSetSupplier*
and *ExpandableStepIterator#setTraverserSet*.
The tests failed because I initially added a pure Supplier which is not
serializable. I fixed the issue and now tests passed.
Not sure if TraversalVertexProgram can have the same benefit so I didn't add
the same logic there - I am not familiar with TraversalVertexProgram but it is
for BigData analysis like Spark right ?
The reason for an internal Id is something OLTP specific (you know, there is a
performance benefit by having a fixed length identifier in some Storage engine
for instance).
Please let me know if there are any test needed are missing or have any
comments on the pull request.
> Can you say which graph database you intend this change for?
You can see my organization in Github...
> TraverserSet should be extendable for GraphDB provider
> ------------------------------------------------------
>
> Key: TINKERPOP-2396
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2396
> Project: TinkerPop
> Issue Type: Improvement
> Components: process
> Reporter: redtree
> Priority: Minor
>
> Currently in many TinkerPop steps, TraverserSet is internally created and
> used.
> However GraphDB provider may want to use their own TraverserSet to change
> the logic how it populates an inner Map.
> Just let me give one example
> Say internally we have a different representation of id (which won't be
> exposed to client when the result is returned) for each element and want to
> use it for hash key.
> Because we can't overwrite ElementHelper's behavior so first we tried
> overriding Element#hashCode but after that we saw that this test effectively
> fails.
>
> [https://github.com/apache/tinkerpop/blob/cff4c161615f2b50bda27b6ba523c7f52b833532/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/util/detached/DetachedVertexTest.java#L75]
> Here v is our own custom vertex instance which uses internal id for hashCode
> but DetachedVertex just uses a user facing id copied from v, so they are not
> considered as equal when used as hash key.
> We may skip this test but it's better to keep all Tinkerpop tests passed as
> GraphDB provider I think.
> So instead, I will propose to change so that we can add a capability to swap
> TraverserSet as needed.
> The change should be simple,
> 1. ExpandableIterator should have constructor like ExpandableIterator(final
> Step<S, ?> hostStep, final TraverserSet<S> traverserSet) and use the
> traverserSet as its own traverserSet value.
> 2. AbstractStep should have construct like AbstractStep(final
> Traversal.Admin traversal, final Supplier<TraverserSet<S>>
> traverserSetSupplier) and use traverserSetSuppler.get() for all places that
> currently we instantiate new TraverserSet.
> Also this supplier.get() may be passed when AbstractStep instantiates
> ExpandableIterator.
> So still GraphDB provider needs to extends all Steps that deals with
> TraverserSet and overwrite their constructor to have their own
> traverserSetSupplier, but after that they can freely use their own
> TraverserSet, so the logic how the key of TraverserSet is determined is up to
> GraphDB provider.
> Or if we add some helper method like getTraverserSetSupplier under, for
> instance, Graph class, then instead of extending all steps we can just
> override that method under our own Graph class. But I am not sure which is
> better in this case, but either way the goal is the same.
> Any thoughts or objections on this idea ?
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
