i was trying to summarize the decisions we're trying to make on this issue
and it occurred to me that the problems we're facing here raise some
fundamental questions about the original direction i'd proposed. i long ago
chose a strategy approach to doing this, mostly because we have some
history of doing that with HaltedTraverserStrategy and
ReferenceElementStrategy, but I can't help wondering now if that pattern
should be followed. It's troubling me that DetachStrategy,
ReferenceElementStrategy , etc. really don't have any relevance to embedded
use cases (aside from the very specific case of OLAP with
HaltedTraverserStrategy). They are really just mechanisms to help control
serialization costs and as such are more request/response oriented as
opposed to traversal oriented. If we treated it that way maybe some of
these hard decisions would go away.
I was thinking that maybe we could add “detachment” to RequestOptions and
use the same syntax already established for DetachStrategy in there. Hooked
up properly this would allow something like:
g.with(‘detach’, Detach.custom("name", "age")).V()
On the server, we’d then just have to extract that Detach from the
RequestMessage and apply it on serialization as needed.
I see several advantages with this revised design:
1. No more worries about the extra step in profile() - which is even a
problem now in ReferenceElementStrategy
2. This works as easily for scripts as it does for bytecode
3. We don’t mix this capability up with embedded use cases, so there’s no
confusion about when/why you use this.
Going further with the above Gremlin syntax, maybe the argument to with()
should be like:
g.with(‘detach’, "ALL").V()
g.with(‘detach’, "NONE").V()
g.with(‘detach’, ["name", "age"]).V() // a list implies CUSTOM
The advantage here is that we don’t have to change the grammar in
gremlin-language and the serializers don’t need to change at all. We could
retain the Detach.custom("name", "age") or similar syntax for Java/Groovy
and come up with similar sugar for the other languages.
I think that still leaves the question of what the default should be on the
server for 3.7.0 - perhaps we come back to that once there is feedback on
this design revision.
On Tue, Nov 22, 2022 at 7:06 AM Stephen Mallette <spmalle...@gmail.com>
wrote:
> > My proposal is to always have some detachment strategy in server
> configuration and add `DetachedStrategy` to allow users the flexibility to
> customize the result.
>
> I think default detachment on the server should remain "reference" so if
> you can do that with your new DetachStrategy then that is preferred with
> the idea that HaltedTraverserStrategy is no longer useful and can be
> deprecated.
>
> > How to handle more than one DetachedStrategy?
>
> I agree that they shouldn't stack - DetachedStrategy should not be used
> for some sort of security use case. A client-side one should replace the
> server-side one.
>
> > Another question is whether or not to show `DetachElementStep` in the
> response for profiling requests like `g.V().profile()`.
>
> This is the hardest question and goes back to the first because you will
> be using DetachStrategy by default on the server side and, as a result,
> when the user does their g.V().profile() without using DetachStrategy they
> will see your DetachElementStep even though they didn't add one on the
> client side. Right now we hide the detachment behind
> HaltedTraverserStrategy (for bytecode based requests) and in the default
> configuration of ReferenceElementStrategy in the startup script. The latter
> makes the former a bit redundant actually.
>
> It seems that for script based requests we see ReferenceElementStep now in
> the profile():
>
> gremlin> :> g.V().profile()
> ==>Traversal Metrics
> Step Count
> Traversers Time (ms) % Dur
>
> =============================================================================================================
> TinkerGraphStep(vertex,[]) 6
> 6 0.623 37.21
> ReferenceElementStep 6
> 6 1.052 62.79
> >TOTAL -
> - 1.676 -
>
> I don't particularly like that but it might be helpful to see that the
> detachment is explicitly happening by way of a step.This line of thinking
> has me wondering if we should remove the default detachment all together
> from Gremlin Server configuration:
>
> 1. Most folks should not be relying on ReferenceElementStrategy to protect
> them from fat elements, because we've spent years pushing them toward
> valueMap(), project(), etc. If they suddenly started getting properties on
> elements I sense that wouldn't affect many people negatively.
> 2. With ease of use and intuitive behavior in mind, new users will expect
> properties to be on their elements.
> 3. Removing default detachment would make it so that the DetachElementStep
> only appears when the user explicitly sets it themselves. There would be no
> surprise that it is there in the profile().
> 4. The notion of default detachment would then fall to the serializers
> which would just include all properties in a DetachedElement.
>
> If TinkerPop took this route then we'd have to be sure to warn users of
> (1) possible increased serialization times if they were relying on
> elements having no properties and (2) the fact that if they wrote their
> code to explicitly use a ReferenceVertex they will need to add "reference"
> detachment or generalize their code to the Element interfaces. Maybe some
> other stuff too?
>
>
>
>
>
>
>
> On Fri, Nov 18, 2022 at 5:30 PM Valentyn Kahamlyk
> <valent...@bitquilltech.com.invalid> wrote:
>
>> Hi everyone,
>>
>> I'm working on properties on elements (
>> https://issues.apache.org/jira/browse/TINKERPOP-2824).
>> I found HaltedTraverserStrategy` in `TraverserIterator` that does about
>> the
>> same, converting Elements to detached or references. Tests use reference
>> Elements, and detached is default for production. GLV's possibly can
>> override strategy to get different results.
>> My implementation of `DetachedStrategy` is to add the last step to the
>> traversal which modifies the result, so, technically g.V() becomes
>> g.V().[DetachStep].
>> Also default server configuration adds ReferenceElementStrategy
>>
>> https://github.com/apache/tinkerpop/blob/master/gremlin-server/scripts/empty-sample.groovy#L45
>> to each traversal.
>>
>> 1. So I end up with 3 ways to cut properties and I'm not sure how to
>> handle
>> it. What options would you suggest?
>> My proposal is to always have some detachment strategy in server
>> configuration and add `DetachedStrategy` to allow users the flexibility to
>> customize the result. `HaltedTraverserStrategy` can be marked deprecated
>> as
>> it handles only bytecode traversal, but not used for scripts.
>> Another side of the problem is the need to detach custom vertex
>> implementations like `TinkerVertex`.
>>
>> 2. How to handle more than one `DetachedStrategy`? For example one can be
>> set in Gremlin Server configuration and other in user traversal.
>> Options:
>> a) apply all. Each successive DetachStrategy becomes more restrictive.
>> b) apply only last one. Use case: there are default server configurations
>> to return only some properties and the user can override it for each
>> request.
>> I think both options make sense, but I prefer the second.
>>
>> 3. Another question is whether or not to show `DetachElementStep` in the
>> response for profiling requests like `g.V().profile()`. Users may be
>> confused at the appearance of a step that they did not add, but on the
>> other hand it will be an explanation why there are no properties in the
>> response.
>> Is it better to hide `DetachElementStep` or vice versa?
>>
>> Thanks, Valentyn
>>
>
