It would be nice to get to some consensus on this thread so we could put
something out for review on Gremlin Users and ultimately VOTE. I've
gathered from this thread that:
1. It seems like there is some consensus on typing JSON non-natives only
(as opposed to everything). Embedding a type for existing JSON types would
be more consistent, but it seems redundant. I suppose we could hang that
out there for public opinion on the gremlin-users list as i'm not sure what
I would like here. I'm personally not a consumer of GraphSON with embedded
types (especially off the jvm) so I'd rely on the drivers devs and the
greater user community to guide the vote i cast there.
2. The general structure of the revised GraphSON that Kevin proposed seems
to be generally accepted and I think using the non-canonical name for the
types makes sense, though I suppose we leave open the chance for collision
under that model. Perhaps there is a configuration we could add there.
3. Dylan wondered: "It still feels a little too half hearted to warrant a
new serializer version though?" I don't see how we can't stamp out a new
version of GraphSON if we do this. The format for embedded types is
changing (untyped will stay the same). Unless there is a way to make this a
configuration of the current version so that it can easily read both
formats? maybe this is all just configuration of the same GraphSON format?
you have the following configuration options:
a. JSON native
b. full embedded type with canonical name
c. full embedded type with simple name
b. partial embedded type with canonical name
c. partial embedded type with simple name
....hmm even with those options you break version because both b and c
options (which would be represent complete non-lossy approach wouldn't
match the format we have now (as the current format doesn't type json
primitives) anyway, open to ideas on how we could do this without having to
version GraphSON....
On Sat, Apr 9, 2016 at 5:04 PM, Dylan Millikin <dylan.milli...@gmail.com>
wrote:
> The double map comment if I recall correctly was in the case of typing
> everything (even JSON natives). So typing a map would be the actual case.
>
> At this point I think it would be good to make sure whichever model we pick
> prevents as much type loss as possible before moving forward. Consistency +
> Java classes + JSON non-natives is definitely a convenience but we might as
> well go the extra mile if we're going to add a v2 serializer (if - as it
> should go through a DISCUSS/VOTE).
>
> On Thu, Apr 7, 2016 at 3:57 PM, Kevin Gallardo <gallardo.kev...@gmail.com>
> wrote:
>
> > > - List, like suggested in the original thread. Pros: simple. Cons: if
> > the>
> > > generate List has two maps your un-serializer will need to look ahead
> > into>
> > > the maps to see if one has a "@class" key as we can't guaranty List
> > order.>
> >
> > Sorry but I don't see, with the model proposed, how can the List contain
> 2
> > maps with one being the map with the type, I can't manage to find an
> > example, but if you have one in mind, please let me know. Moreover, why
> > can't we guaranty the order? GraphSON serializers methods explicitly call
> > the type serializers before serializing the values, and since it is a
> list,
> > the order is conserved.
> > I am not against the Map approach, but I don't see a valid reason not to
> > use the list, and it seems less verbose..
> >
> > I agree with your conclusion, seems like a reasonable decision for Java
> > classes + JSON non-natives. Just to clarify, you mention overhead for the
> > server serializers/GraphSONWriter with Generic names, I am not sure it
> > applies though, the intention originally would be to encode only the
> Class
> > name rather than the canonical name. i.e. for example from
> > "java.time.Instant" to "Instant", and this does not imply overhead on the
> > serialization process.
> >
> > > > I'd ask for the same question concerning typing on natively supported
> > JSON types, is the overhead worth it?
> > > I think that there have been a number of struggles for non-jvm
> languages
> > around discerning float/double and int/long with respect to graphson.
> >
> > Indeed, with this proposal, since the typing would work both ways
> > (ser/de), the GraphSONReader would be able to coerce automatically an
> > Element having non-JSON-native properties into the correct type, from
> > reading the correctly typed JSON.
> >
> > Thanks for your answer.
> >
> > On 2016-04-05 16:03, Dylan Millikin <d...@gmail.com> wrote:
> > > At this point I'm more or less neutral here. As a recap, I guess there
> > are>
> > > two points:>
> > >
> > > 1) Making the graphSON typing more homogenous across types (Map vs List
> > vs>
> > > Long), no special cases. Here the values could either be encapsulated
> in
> > :>
> > > - List, like suggested in the original thread. Pros: simple. Cons: if
> > the>
> > > generate List has two maps your un-serializer will need to look ahead
> > into>
> > > the maps to see if one has a "@class" key as we can't guaranty List
> > order.>
> > > It doesn't imply much overhead though.>
> > > - Map. Pros: more thorough / theoretically a tad faster on the client
> > end.>
> > > Cons: more verbose, less readable.>
> > >
> > > 2) What to type and how to type it (specific Java class vs generic
> > naming,>
> > > etc.),>
> > >
> > > How to print the types:>
> > >
> > > - As java classes, Pros: Are self explanatory, native to the
> serializing>
> > > end so no overhead. Cons: implies systematic overhead on the client end
> > for>
> > > non-jvm mapping.>
> > > - As generic names, Pros: non-jvm mapping can cherry pick which types
> it>
> > > needs to map so less overhead. Cons: specific documentation is
> required,
> > i>
> > > guess that theoretically there's more overhead for the server end>
> > > serializer as well.>
> > >
> > > What to type:>
> > >
> > > - Everything. Pros: if added to 1) it would make GraphSON very
> > homogenous>
> > > and fail proof. Cons: overhead overhead overhead (need to figure out
> if>
> > > it's significant or not)>
> > > - JSON non-natives only. Pros: less overhead, simplifies the current>
> > > implementation. Cons: potentially not as safe. Though I suspect that
> any>
> > > language supporting JSON should have workarounds for some of the
> > pitfalls.>
> > > So probably a non-issue. Doesn't help in solving any of the int/long
> and>
> > > float/double issues though.>
> > >
> > >
> > > Conclusion:>
> > >
> > > Looking at the above I guess that implementing 1) (either choice) and
> 2)>
> > > Java classes + JSON non-natives. Provides a simpler and easier to use>
> > > GraphSON representation with equal usability to the current
> > implementation.>
> > > It still feels a little too half hearted to warrant a new serializer>
> > > version though?>
> > >
> > > On Tue, Apr 5, 2016 at 4:07 PM, Stephen Mallette <sp...@gmail.com>>
> > > wrote:>
> > >
> > > > > I'd ask for the same question concerning typing on natively
> > supported>
> > > > JSON types, is the overhead worth it?>
> > > >>
> > > > I think that there have been a number of struggles for non-jvm
> > languages>
> > > > around discerning float/double and int/long with respect to
> graphson.>
> > > > That's been true of TinkerPop 2.x and 3.x. We did a better job in
> 3.x
> > by>
> > > > pushing some of the responsibility for id parsing (for graphs that
> use>
> > > > numeric types for ids) down to the Graph implementation level so that
> > has>
> > > > been helpful, but there's nothing for situations like:>
> > > >>
> > > > v.property("someFloatField", x)>
> > > >>
> > > > we typically recommend that users coerce values as:>
> > > >>
> > > > v.property("someFloatField", Float.valueOf(x))>
> > > >>
> > > > or just don't use float in your schema. I'm not saying the overhead
> > of>
> > > > this is worth what we'd get, but just pointing out that there is a
> > commonly>
> > > > seen problem to solve here. It might be fine to stick with our
> current>
> > > > advice on implementations patterns for this. We might want to raise
> > this>
> > > > issue on gremlin-users for more feedback once we have better
> consensus>
> > > > here.>
> > > >>
> > > >>
> > > >>
> > > > On Mon, Apr 4, 2016 at 12:03 PM, Kevin Gallardo <>
> > > > kevin.galla...@datastax.com>
> > > > > wrote:>
> > > >>
> > > > > Thank you for your answers.>
> > > > >>
> > > > > I agree with the reasoning of needing the full types names when it
> > comes>
> > > > > to check the specification, however after you saw the specs and you
> > know>
> > > > > how to deserialize it, is it worth it to still get the full types
> in
> > all>
> > > > > the server's responses afterwards?>
> > > > >>
> > > > > I'd ask for the same question concerning typing on natively
> > supported>
> > > > JSON>
> > > > > types, is the overhead worth it? If you only want to be able to>
> > > > distinguish>
> > > > > non native types from the native ones.>
> > > > > It could also maybe be possible to have some kinds of "level" for
> > typing>
> > > > > then, "no type", "only custom types", "all types". What do you
> > think? (I>
> > > > am>
> > > > > not sure how typing native JSON elements is supported by Jackson
> > though).>
> > > > >>
> > > > > Cheers.>
> > > > >>
> > > > > On 2016-04-01 14:56, Dylan Millikin <d....@gmail.com> wrote:>
> > > > > > Ok I see. That might be a little redundant but it definitely has
> > it's>>
> > > > > > advantages.>>
> > > > > > My initial thought was mostly like Kevin's regarding JSON native
> > types.>
> > > > > I>>
> > > > > > had a few reservations regarding the type naming and I've given
> > this a>
> > > > > bit>>
> > > > > > more thought :>>
> > > > > >>
> > > > > > I think the java specific typing is a good thing because it
> > directly>>
> > > > > > references those types as being java types (with those specs).
> > Typing>
> > > > in>>
> > > > > > other languages do not follow any standard, "int" can mean
> > anything>
> > > > > ranging>>
> > > > > > from "short" to "long" to nothing depending on your language and>
> > > > system.>>
> > > > > > PHP is horrible for this, int = int on 32b systems and int = long
> > on>
> > > > 64b>>
> > > > > > systems, and I believe other languages also have similar issues
> > (the>
> > > > > Ariane>>
> > > > > > space program notoriously lost one of their rockets to this kind
> > of>
> > > > > typing>>
> > > > > > issue). So drivers/clients -should- map the types anyways and
> the>
> > > > > current>>
> > > > > > implementation makes it easy to understand where to go for
> specs.>>
> > > > > >>
> > > > > > If anything, considering the above, you may actually want to type
> > JSON>>
> > > > > > native types as well to make this completely fail-proof.>>
> > > > > >>
> > > > > > This goes in the completely opposite direction of what the OP>
> > > > suggested>>
> > > > > > though. Thoughts?>>
> > > > > >>
> > > > > >>
> > > > > > On Fri, Apr 1, 2016 at 3:17 PM, Stephen Mallette <
> sp...@gmail.com
> > >>>
> > > > > > wrote:>>
> > > > > >>
> > > > > > > I haven't yet had a chance to fully think this change through,
> > but I>
> > > > > think>>
> > > > > > > the idea would be to produce a new version of GraphSON and keep
> > the>
> > > > > old>>
> > > > > > > version around for backward compatibility. It would also only
> be
> > a>
> > > > > breaking>>
> > > > > > > change for type embedded GraphSON, which i would guess the
> > minority>
> > > > > are>>
> > > > > > > using right now as its verbosity and dependence on java types
> > makes>
> > > > it>>
> > > > > > > borderline unusable off the JVM. I think this is a nice
> > proposal>
> > > > > because>>
> > > > > > > it tries to make embedded types something that could actually
> be>
> > > > > usable in>>
> > > > > > > a more general fashion across different programming languages.>
> > > > Anyway,>
> > > > > the>>
> > > > > > > key point is that we would want to try to still support the
> old>
> > > > > version of>>
> > > > > > > type-embedded graphson in addition to what kevin has
> proposed.>>
> > > > > > >>>
> > > > > > > On Fri, Apr 1, 2016 at 9:11 AM, Dylan Millikin <
> dy...@gmail.com
> > >>>
> > > > > > > wrote:>>
> > > > > > >>>
> > > > > > > > I also thought about this when I tried to set this serializer
> > up>
> > > > > with the>>
> > > > > > > > php driver. This is quite a breaking change though.>>
> > > > > > > >>>
> > > > > > > > On Thu, Mar 31, 2016 at 1:33 PM, Kevin Gallardo <>>
> > > > > > > > kevin.galla...@datastax.com>>
> > > > > > > > > wrote:>>
> > > > > > > >>>
> > > > > > > > > Hi,>>
> > > > > > > > >>>
> > > > > > > > >>>
> > > > > > > > > By working closely with Tinkerpop I have been interacting
> > quite a>
> > > > > bit>>
> > > > > > > > with>>
> > > > > > > > > the GraphSON format and I have come up with the need of
> > using>
> > > > the>>
> > > > > > > > > "embedTypes" option of the GraphSON Mapper to get more
> > insight of>
> > > > > the>>
> > > > > > > > types>>
> > > > > > > > > of the data I was transferring through the GraphSON
> > payload.>>
> > > > > > > > >>>
> > > > > > > > > By default vertices, edges, having properties that have
> > another>
> > > > > type>>
> > > > > > > than>>
> > > > > > > > > the natively supported types in the JSON format are encoded
> > as>
> > > > > their>>
> > > > > > > > String>>
> > > > > > > > > representation. Meaning that a Vertex property being a UUID
> > will>
> > > > > look>>
> > > > > > > > like>>
> > > > > > > > > "24B7DED2-F72A-11E5-815D-79DF61FECC63" which, for the user>
> > > > > consuming>>
> > > > > > > the>>
> > > > > > > > > JSON produced by the GraphSONWriter, will not be
> > distinguishable>
> > > > > from>>
> > > > > > > > being>>
> > > > > > > > > a String or a UUID. That is usually why you need to
> activate
> > the>
> > > > > type>>
> > > > > > > > > embedding of GraphSON. Documentation about the current
> > output of>>
> > > > > > > GraphSON>>
> > > > > > > > > typing :>>
> > > > > > > > >>>
> > > > > > > > >>>
> > > > > > > >>>
> > > > > > >>
> > > > >>
> > > >
> >
> http://tinkerpop.apache.org/docs/3.1.1-incubating/reference/#graphson-reader-writer
> > >
> > > > > >>
> > > > > > > > >>>
> > > > > > > > > However, right now, embedding types with GraphSON comes
> with
> > a>
> > > > few>>
> > > > > > > trade>>
> > > > > > > > > offs :>>
> > > > > > > > >>>
> > > > > > > > > - It is very verbose, mostly because Arrays and Maps
> > types>
> > > > are>>
> > > > > > > > embedded.>>
> > > > > > > > > But Arrays and Maps are natively supported in JSON, and
> > other>
> > > > > native>>
> > > > > > > > > types>>
> > > > > > > > > like Strings or booleans do not appear typed, so these
> > types>
> > > > > should>>
> > > > > > > > > logically also be excluded, since natively supported in
> > JSON.>>
> > > > > > > > > - It is not consistent, if the type is to be embedded in
> > a>
> > > > JSON>>
> > > > > > > > Object,>>
> > > > > > > > > it will come as a JSON Object's element, a key-value
> pair>
> > > > > {"@class">>
> > > > > > > :>>
> > > > > > > > > "java.util.HashMap", ... }, whereas if the type is to
> be>
> > > > > embedded>>
> > > > > > > in a>>
> > > > > > > > > JSON>>
> > > > > > > > > Array, it will be embedded in "best-effort" and put as
> a>
> > > > simple>>
> > > > > > > value>>
> > > > > > > > > that>>
> > > > > > > > > will be ["java.util.HashMap", ... ], hence it makes it>
> > > > > difficult to>>
> > > > > > > > > automatically parse typed results.>>
> > > > > > > > > - It is Java centric.>>
> > > > > > > > >>>
> > > > > > > > > I'd like to propose a format to encapsulate values format,
> > that>
> > > > > would>>
> > > > > > > > > address the points mentioned above, in which the main idea
> > being>
> > > > > that>>
> > > > > > > > > whenever a type needs to be embedded, the value itself and
> > the>
> > > > > type>>
> > > > > > > would>>
> > > > > > > > > be encapsulated in an Array, and the first element is a
> JSON>
> > > > > Object>>
> > > > > > > > > containing only the key/value pair {"@class": "TypeName"},
> > the>
> > > > > second>>
> > > > > > > > > element being the value.>>
> > > > > > > > >>>
> > > > > > > > > A Short integer value encoded would change from :>>
> > > > > > > > >>>
> > > > > > > > > 2>>
> > > > > > > > >>>
> > > > > > > > > *untyped, to :>>
> > > > > > > > >>>
> > > > > > > > > [{"@class":"Short"}, 2]>>
> > > > > > > > >>>
> > > > > > > > > with the type.>>
> > > > > > > > >>>
> > > > > > > > > It's just an example, but I thought this format would be
> > robust>
> > > > > enough.>>
> > > > > > > > And>>
> > > > > > > > > properties or other elements for which types are natively>
> > > > > supported in>>
> > > > > > > > JSON>>
> > > > > > > > > would not need the additional metadata about their types,
> > like it>
> > > > > is>>
> > > > > > > now>>
> > > > > > > > > with GraphSON. I have a working prototype for this.>>
> > > > > > > > >>>
> > > > > > > > > Taking the Tinkerpop's doc example of GraphSON embedded
> > types>
> > > > and>>
> > > > > > > > applying>>
> > > > > > > > > the proposed changes :>>
> > > > > > > > >>>
> > > > > > > > > ----------------------------->>
> > > > > > > > >>>
> > > > > > > > > {>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "id":1,>>
> > > > > > > > > "label":"person",>>
> > > > > > > > > "outE":{>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "created":[>>
> > > > > > > > > "java.util.ArrayList",>>
> > > > > > > > > [>>
> > > > > > > > > {>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "id":9,>>
> > > > > > > > > "inV":3,>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "weight":0.4>>
> > > > > > > > > }>>
> > > > > > > > > }>>
> > > > > > > > > ]>>
> > > > > > > > > ],>>
> > > > > > > > > "knows":[>>
> > > > > > > > > "java.util.ArrayList",>>
> > > > > > > > > [>>
> > > > > > > > > {>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "id":7,>>
> > > > > > > > > "inV":2,>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "weight":0.5>>
> > > > > > > > > }>>
> > > > > > > > > },>>
> > > > > > > > > {>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "id":8,>>
> > > > > > > > > "inV":4,>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "weight":1>>
> > > > > > > > > }>>
> > > > > > > > > }>>
> > > > > > > > > ]>>
> > > > > > > > > ]>>
> > > > > > > > > },>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "name":[>>
> > > > > > > > > "java.util.ArrayList",>>
> > > > > > > > > [>>
> > > > > > > > > {>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "id":[>>
> > > > > > > > > "java.lang.Long",>>
> > > > > > > > > 0>>
> > > > > > > > > ],>>
> > > > > > > > > "value":"marko">>
> > > > > > > > > }>>
> > > > > > > > > ]>>
> > > > > > > > > ],>>
> > > > > > > > > "age":[>>
> > > > > > > > > "java.util.ArrayList",>>
> > > > > > > > > [>>
> > > > > > > > > {>>
> > > > > > > > > "@class":"java.util.HashMap",>>
> > > > > > > > > "id":[>>
> > > > > > > > > "java.lang.Long",>>
> > > > > > > > > 1>>
> > > > > > > > > ],>>
> > > > > > > > > "value":29>>
> > > > > > > > > }>>
> > > > > > > > > ]>>
> > > > > > > > > ]>>
> > > > > > > > > }>>
> > > > > > > > > }>>
> > > > > > > > >>>
> > > > > > > > >>>
> > > > > > > > >
> > -------------------------------------------------------------->>
> > > > > > > > >>>
> > > > > > > > > With the changes :>>
> > > > > > > > >>>
> > > > > > > > > {>>
> > > > > > > > > "id":1,>>
> > > > > > > > > "label":"person",>>
> > > > > > > > > "outE":{>>
> > > > > > > > > "created":[>>
> > > > > > > > > {>>
> > > > > > > > > "id":9,>>
> > > > > > > > > "inV":3,>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "weight":0.4>>
> > > > > > > > > }>>
> > > > > > > > > }>>
> > > > > > > > > ],>>
> > > > > > > > > "knows":[>>
> > > > > > > > > {>>
> > > > > > > > > "id":7,>>
> > > > > > > > > "inV":2,>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "weight":0.5>>
> > > > > > > > > }>>
> > > > > > > > > },>>
> > > > > > > > > {>>
> > > > > > > > > "id":8,>>
> > > > > > > > > "inV":4,>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "weight":1>>
> > > > > > > > > }>>
> > > > > > > > > }>>
> > > > > > > > > ]>>
> > > > > > > > > },>>
> > > > > > > > > "properties":{>>
> > > > > > > > > "name":[>>
> > > > > > > > > {>>
> > > > > > > > > "id":[>>
> > > > > > > > > {"@class":"Long"},>>
> > > > > > > > > 0>>
> > > > > > > > > ],>>
> > > > > > > > > "value":"marko">>
> > > > > > > > > }>>
> > > > > > > >
> > [message truncated...]
> >
> >
> > —
> > Kevin Gallardo
> > kgdo.me
> >
> >
>
