tinkerpop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dylan Millikin <dylan.milli...@gmail.com>
Subject Re: GraphSON improvement proposal
Date Tue, 05 Apr 2016 15:03:38 GMT
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 <spmallette@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.gallardo@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.gallardo@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>
> > > > > >>
> > > > >>
> > > >>
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message