[TINKERPOP-2234] - Proposal for Type Predicate & Type Enum

[xiazcy]

As discussed previously (see links in the Jira), this proposal aims to add a typeOf predicate for asserting types in Gremlin.

For maintainability, this also defines a set of core type enum (GType) to be used in Gremlin (which will replace N for asNumber()), as well as an overload for String tokens, defined by a global cache for providers to register their own custom types, and to be recognized by the Grammar.

VOTE +1

[spmallette]

could you please include the hard line breaks?

note a few spelling mistakes: consiste, singel, extensable...please review

[spmallette]

I guess this is implemented like Text enum. Wouldn't it be PBiPredicate<Object, GType>? I assume the various P.typeOf() overloads would normalize to a GType or is that not how it would work?

[xiazcy]

Hmmm I might not understand how providers extend these, but I'd think if we are trying to test classes not defined in GType but registered into the cache (i.e. provider-defined), then they would not be able to normalize to GType? So we'd need to keep this <Object, Object> and assert types inside the implementation?

[spmallette]

Maybe we only need to think about Java here as Type isn't really exposed to users. Like, it's PBiPredicate<Object,Class<?>> where we normalize to a Java Class, roughly:

    public static <V> P<V> typeOf(final Class<?> value) {
        return new P(new PBiPredicate<V, Class<?>>() {
            @Override
            public boolean test(V v, Class<?> aClass) {
                return v.getClass().equals(aClass);
            }
        }, value);
    }

that captures all the typeOf variations that we have, while constraining P to more than just Object.

while we consider this syntax "sugar" to users because it's not available to all GLVs, it's actually better described as a feature of embedded Gremlin. perhaps that distinction needs to be an angle that is considered when adding new steps. we'd thus have considerations for:

• canonical
• embedded
• sugar
  Addressing those as they pertain to each Gremlin implementation would help better isolate designs.

[spmallette]

Maybe this is just a point of clarification but I thought that this was going to be like TraversalStrategy grammar extensions. With the TraversalStrategy implementation the grammar doesn't use strings. You can have a MyProviderStrategy and refer to it directly:

g.withStrategies(MyProviderStrategy)

and even do more complex configured ones like:

g.withStrategies(MyProviderStrategy(config1: true, config2: ['a', 'b']))

I think we want the same thing here. In the grammar if a provider has a org.provider.Point class it is registered in the cache and you can just do typeOf(Point) in the grammar.

In Java if you have a Point class to be able to do g.inject(new Point(1,2)) then why not just typeOf(Point.class) in Java which converts internally to typeOf('org.provider.Point') for execution in the grammar.

In cases where there is no provider class, like maybe .NET, users are just going to have to use the string form: TypeOf('org.provider.Point')

I do think this section however does raise an interesting point about name collisions. I largely ignored that for TraversalStrategy sticking to keying the registry on the simple name for the class as chance of collisions seemed low. For types that might not be the case as I think I could see TinkerPop having a Point and JanusGraph having a Point. Perhaps though we use convention to handle this. TinkerPop retains the global namespace as a priority and providers can fit there stuff in around our naming. If they conflict, providers must prefix their types. So we would have Point and JanusGraph would have jx:Point. This body of work should clarify this usage in the provider docs.

[xiazcy]

The thought here was to give freedom in defining tokens for grammar to recognize instead of using the default simple name. I had it originally when I was considering provider-defined enum tokens with customized prefixes (e.g. CType.XXX), which would also avoid name collisions.

[spmallette]

I'm not sure, but that feels like it opens more doors than we know we need. Let me ask this, how does this align with PDTs then? You use Point in this example. If we open a door to refer to Point as MyPoint for typeOf is there also going to be a chance that users will refer to it as Point in their code but then MyPoint in the grammar?

[spmallette]

I assume searches over the Java class name are restricted to the registry? in other words, you could do typeOf("java.awt.Color") but unless it is registered, we won't instantiate the Class for comparison, right? it would be good to clarify that here and in future docs.

[xiazcy]

Currently I'm thinking of instantiating the object regardless, but it does make more sense to only do so if registered.

[spmallette]

i think it should only be if its registered. it reduces the chance of someone doing something foul from a security perspective. i don't know what foul would be but no reason to leave a hole where there doesnt need to be one. please make sure that's clear in the proposal. thanks.

[spmallette]

please add an alternative example where its doing P.or(...) with typeOf

[spmallette]

So asNumber will take a GType in addition to N? what happens if asNumber takes a GType that isn't referencing a numeric? filter i guess?

I've never been sure i like that we have different ways of referencing a number: GType and N. As I look at this example in particular, i like it less.

// the exact example mixes a few things with asNumber(GType) and typeOf(Class) from the java sugar. 
// maybe people would do that....
g.V().hasLabel('person').
  sideEffect(property('age', values('age').asNumber(GType.DOUBLE))).
  values('age').is(typeOf(Double))

// approaching strictly from the grammar and GType we'd have the following which is nice and symmetric
g.V().hasLabel('person').
  sideEffect(property('age', values('age').asNumber(DOUBLE))).
  values('age').is(typeOf(DOUBLE))

// approaching strictly from the Java sugar its super nice - incidentally, i don't see the
// asNumber(Class<? extends Number>) overload, but we can see how nice that looks!!
g.V().hasLabel('person').
  sideEffect(property('age', values('age').asNumber(Double))).
  values('age').is(typeOf(Double))

// approaching strictly from the grammar and where someone uses N and GType with the 
// canonical Gremlin as defined so far. 
g.V().hasLabel('person').
  sideEffect(property('age', values('age').asNumber(double_))).
  values('age').is(typeOf(DOUBLE))

That last example seems problematic to me, particularly for those writing scripts. The competing types are going to be confusing in that case. Like, why do i sometimes use double_ and other times DOUBLE? You could see folks mixing them up quite easily.

[xiazcy]

The current thought is to replace the N enum entirely, and only have GType. For asNumber() there will be a check for numerical GType tokens and throw exception in this case.

If we want to keep N, the tokens would either be entirely the same (i.e. DOUBLE) as GType, or GType will not contain what N has, for the reason of the last example.

[spmallette]

I think the proposal should be updated with this information.

For asNumber() there will be a check for numerical GType tokens and throw exception in this case.

runtime exception or traversal construction exception?

If we want to keep N

Is there some idea to keep N still? if so, why?

[andreachild]

Is there some idea to keep N still? if so, why?

I would not think there is a reason to keep N

[xiazcy]

runtime exception or traversal construction exception?

Traversal construction, unless there are objections on throwing there.

Is there some idea to keep N still? if so, why?

No, I would not keep N. It had been brought up as an idea that we can keep the types clean for asNumber() for compile time, but I'd think it's unnecessary to have separate enums, so the current consensus is to keep GType only.

[andreachild]

The examples below mention GType.NUMBER, GType.VERTEX, should those be added here?

[xiazcy]

Yes, I took a subset to show as example, I didn't want to bloat the content here so the full spec is down in Appendix.

[spmallette]

please include a comment in the sample code here so that folks know to look in the appendix

[andreachild]

Is this example and the ones below missing the .class ie. typeOf(Number.class)?

[spmallette]

in java you need the .class, but groovy is fine without. might be worth distinguishing that and as i'd alluded to elsewhere to try to not mix syntax .