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 <spmalle...@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"> > > > > > > }> > > > > > > ],> > > > > > > "age":[> > > > > > > {> > > > > > > "id":[> > > > > > > {"@class":"Long"},> > > > > > > 1> > > > > > > ],> > > > > > > "value":29> > > > > > > }> > > > > > > ]> > > > > > > }> > > > > > > }> > > > > > >> > > > > > > ----------------------------------------------------> > > > > > >> > > > > > > Open to suggestions and your feedback.> > > > > > >> > > > > > >> > > > > > >> > > > > > > Cheers!> > > > > > >> > > > > > > --> > > > > > > KÈvin Gallardo.> > > > > > > kgdo.me> > > > > > >> > > > > >> > > > >> > > > > > >