You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by "Dan LaRocque (JIRA)" <ji...@apache.org> on 2016/08/04 04:23:20 UTC
[jira] [Updated] (TINKERPOP-1397) StarVertex self edge has buggy
interaction with graph filters
[ https://issues.apache.org/jira/browse/TINKERPOP-1397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dan LaRocque updated TINKERPOP-1397:
------------------------------------
Description:
When StarVertex adds a self-loop, it adds an instance of concrete type {{StarOutEdge}} to both its {{outEdges}} map and its {{inEdges}} map.
https://github.com/apache/tinkerpop/blob/8f7218d53a31cf41f4a0269d64ac1c27dfc0907a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java#L321-L330
(line 329 adds a {{StarOutEdge}} to the {{inEdges}} map)
Having a {{StarOutEdge}} in the {{inEdges}} map mostly doesn't matter. However, this does matter in the {{applyGraphFilter}} method. This method uses {{instanceof StarOutEdge}} to guess whether an edge's original direction was in or out:
https://github.com/apache/tinkerpop/blob/8f7218d53a31cf41f4a0269d64ac1c27dfc0907a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java#L470
If you trace {{applyGraphFilter}} through, you see that a {{StarVertex}} with a self-loop can pop out of {{applyGraphFilter}} with two out edges corresponding to the self loop and no in edges. This leads to weird traversal results. One way to trigger this is to write a {{GraphFilterAware}} {{InputRDD}} that produces {{StarVertex}} instances and then run a {{g.V()....inE("somelabel")}}, so that TP pushes down the "somelabel" constraint, which is how I got here.
I think {{addEdge}} should continue putting a {{StarOutEdge}} into {{outEdges}}, like it already does. -However, it should put a {{StarInEdge}} into {{inEdges}}. This would increase the object overhead associated with StarVertices that have self loops (two edge objs instead of one). If we wanted to be obsessive about optimizing this case we could probably pervert the {{inEdge}}/{{outEdge}} datastructure contents to do it, but IMHO that's not worth it.- Actually, I'm no longer convinced that's safe, since I think it would alter some of StarVertex's other semantics. For one, I think retrieving an edge and setting a property on it would probably break.
I'll make a PR soon.
I don't know all of the versions this affects, but I do know it affects 3.2.1.
was:
When StarVertex adds a self-loop, it adds an instance of concrete type {{StarOutEdge}} to both its {{outEdges}} map and its {{inEdges}} map.
https://github.com/apache/tinkerpop/blob/8f7218d53a31cf41f4a0269d64ac1c27dfc0907a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java#L321-L330
(line 329 adds a {{StarOutEdge}} to the {{inEdges}} map)
Having a {{StarOutEdge}} in the {{inEdges}} map mostly doesn't matter. However, this does matter in the {{applyGraphFilter}} method. This method uses {{instanceof StarOutEdge}} to guess whether an edge's original direction was in or out:
https://github.com/apache/tinkerpop/blob/8f7218d53a31cf41f4a0269d64ac1c27dfc0907a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java#L470
If you trace {{applyGraphFilter}} through, you see that a {{StarVertex}} with a self-loop can pop out of {{applyGraphFilter}} with two out edges corresponding to the self loop and no in edges. This leads to weird traversal results. One way to trigger this is to write a {{GraphFilterAware}} {{InputRDD}} that produces {{StarVertex}} instances and then run a {{g.V()....inE("somelabel")}}, so that TP pushes down the "somelabel" constraint, which is how I got here.
I think {{addEdge}} should continue putting a {{StarOutEdge}} into {{outEdges}}, like it already does. However, it should put a {{StarInEdge}} into {{inEdges}}. This would increase the object overhead associated with StarVertices that have self loops (two edge objs instead of one). If we wanted to be obsessive about optimizing this case we could probably pervert the {{inEdge}}/{{outEdge}} datastructure contents to do it, but IMHO that's not worth it.
The change I'm suggesting is a one-liner. I'll make a PR soon.
I don't know all of the versions this affects, but I do know it affects 3.2.1.
> StarVertex self edge has buggy interaction with graph filters
> -------------------------------------------------------------
>
> Key: TINKERPOP-1397
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1397
> Project: TinkerPop
> Issue Type: Bug
> Reporter: Dan LaRocque
>
> When StarVertex adds a self-loop, it adds an instance of concrete type {{StarOutEdge}} to both its {{outEdges}} map and its {{inEdges}} map.
> https://github.com/apache/tinkerpop/blob/8f7218d53a31cf41f4a0269d64ac1c27dfc0907a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java#L321-L330
> (line 329 adds a {{StarOutEdge}} to the {{inEdges}} map)
> Having a {{StarOutEdge}} in the {{inEdges}} map mostly doesn't matter. However, this does matter in the {{applyGraphFilter}} method. This method uses {{instanceof StarOutEdge}} to guess whether an edge's original direction was in or out:
> https://github.com/apache/tinkerpop/blob/8f7218d53a31cf41f4a0269d64ac1c27dfc0907a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java#L470
> If you trace {{applyGraphFilter}} through, you see that a {{StarVertex}} with a self-loop can pop out of {{applyGraphFilter}} with two out edges corresponding to the self loop and no in edges. This leads to weird traversal results. One way to trigger this is to write a {{GraphFilterAware}} {{InputRDD}} that produces {{StarVertex}} instances and then run a {{g.V()....inE("somelabel")}}, so that TP pushes down the "somelabel" constraint, which is how I got here.
> I think {{addEdge}} should continue putting a {{StarOutEdge}} into {{outEdges}}, like it already does. -However, it should put a {{StarInEdge}} into {{inEdges}}. This would increase the object overhead associated with StarVertices that have self loops (two edge objs instead of one). If we wanted to be obsessive about optimizing this case we could probably pervert the {{inEdge}}/{{outEdge}} datastructure contents to do it, but IMHO that's not worth it.- Actually, I'm no longer convinced that's safe, since I think it would alter some of StarVertex's other semantics. For one, I think retrieving an edge and setting a property on it would probably break.
> I'll make a PR soon.
> I don't know all of the versions this affects, but I do know it affects 3.2.1.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)