[ 
https://issues.apache.org/jira/browse/TINKERPOP-1520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15685027#comment-15685027
 ] 

ASF GitHub Bot commented on TINKERPOP-1520:
-------------------------------------------

Github user newkek commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/499#discussion_r89010235
  
    --- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
 ---
    @@ -153,13 +152,16 @@ public void serialize(final Edge edge, final 
JsonGenerator jsonGenerator, final
             }
     
             private void writeProperties(final Edge edge, final JsonGenerator 
jsonGenerator) throws IOException {
    -            final Iterator<Property<Object>> elementProperties = normalize 
?
    +            final Iterator<Property<Object>> edgeProperties = normalize ?
                         IteratorUtils.list(edge.properties(), 
Comparators.PROPERTY_COMPARATOR).iterator() : edge.properties();
    -            if (elementProperties.hasNext()) {
    +            if (edgeProperties.hasNext()) {
                     jsonGenerator.writeFieldName(GraphSONTokens.PROPERTIES);
     
                     jsonGenerator.writeStartObject();
    -                elementProperties.forEachRemaining(prop -> 
safeWriteObjectField(jsonGenerator, prop.key(), prop));
    +                while (edgeProperties.hasNext()) {
    +                    final Property<?> property = edgeProperties.next();
    +                    jsonGenerator.writeObjectField(property.key(), 
property.value());
    --- End diff --
    
    I had noticed the discrepancy in that VertexProperty's Properties are not 
serialized as Properties but instead the values directly are serialized and 
that's an oversight from when I implemented GraphSON2. I understand that the 
same thing is applied here and it makes a smaller footprint but I think we 
should rather fix the VertexProperty's properties to be serialized as Map of 
Properties and not Map of the values, because it reflects directly what is 
going on underneath and so the deserialization process does not need to any 
transformation, just set the fields of the newly created object with the rest 
of the deserialized data, rather than creating. Also it is not consistent 
because each Vertex, Edge, and VertexProperty derive from Element, which has 
the same properties field, a list of ? extends Property so we should come up 
with the same format for all I believe and here a Vertex would have its 
properties serialized objects, but the others would have their properties 
serialized as the values, not sure if that's a major concern but I would 
advocate for consistency as much as possible


> Difference between 'has' step generated graphson2.0 in java and python glv 
> implementation
> -----------------------------------------------------------------------------------------
>
>                 Key: TINKERPOP-1520
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-1520
>             Project: TinkerPop
>          Issue Type: Bug
>          Components: language-variant
>    Affects Versions: 3.2.3
>            Reporter: Andy Tolbert
>
> Noticed that between the java and python implementations, the graphson2.0 
> payload generated for a {{has}} step is different.  i.e. for the given 
> traversal:
> {{g.E().has("weight", 0.2)}}
> The java implementation produces the following graphson:
> {code:javascript}
> {"@type":"g:Bytecode","@value":{"step":[["E"],["has","weight",{"@type":"g:P","@value":{"predicate":"eq","value":{"@type":"g:Double","@value":0.2}}}]]}}
> {code}
> where the python implementation produces the following:
> {code:javascript}
>  {"@type":"g:Bytecode","@value":{"step":[["E"],["has","weight",0.2]]}}
> {code}
> In the java case, a {{g\:P}} typed (predicate) value is provided, where in 
> the python case that isn't the case.
> I'm assuming the java one is correct (primarily since the graph backend seems 
> to like it and return the expected result).   Should GLV implementations 
> behave this way?  I noticed that {{GraphTraversal#has(String propertyKey, 
> Object value)}} in the java TinkerPop api wraps the value in a predicate 
> ({{P.eq}}) under the covers 
> ([link|https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java#L922])
>  so maybe implementors will need to do the same ([python 
> link|https://github.com/apache/tinkerpop/blob/master/gremlin-python/src/main/jython/gremlin_python/process/graph_traversal.py#L193])?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to