You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by Dylan Millikin <dy...@gmail.com> on 2016/04/01 15:11:37 UTC

Re: GraphSON improvement proposal

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
>

Re: GraphSON improvement proposal

Posted by Dylan Millikin <dy...@gmail.com>.
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
> > >
> >
>

Re: GraphSON improvement proposal

Posted by Stephen Mallette <sp...@gmail.com>.
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
> >
>