You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by okram <gi...@git.apache.org> on 2016/04/29 17:36:55 UTC

[GitHub] incubator-tinkerpop pull request: TINKERPOP-1284: StarGraph does n...

GitHub user okram opened a pull request:

    https://github.com/apache/incubator-tinkerpop/pull/298

    TINKERPOP-1284: StarGraph does not handle self-loops correctly.

    https://issues.apache.org/jira/browse/TINKERPOP-1284
    
    ```
    gremlin> graph = TinkerGraph.open()
    ==>tinkergraph[vertices:0 edges:0]
    gremlin>
    gremlin> v = graph.addVertex()
    ==>v[0]
    gremlin> v.addEdge("self",v)
    ==>e[1][0-self->0]
    gremlin> g = graph.traversal()
    ==>graphtraversalsource[tinkergraph[vertices:1 edges:1], standard]
    gremlin> g.V(0l)
    ==>v[0]
    gremlin>  g.V(0l).bothE()
    ==>e[1][0-self->0]
    ==>e[1][0-self->0]
    gremlin> g.V(0l).outE()
    ==>e[1][0-self->0]
    gremlin> g.V(0l).inE()
    ==>e[1][0-self->0]
    gremlin>
    gremlin>
    gremlin> starGraph = org.apache.tinkerpop.gremlin.structure.util.star.StarGraph.of(v)
    ==>stargraph[starOf:v[0]]
    gremlin> sg = starGraph.traversal()
    ==>graphtraversalsource[stargraph[starOf:v[0]], standard]
    gremlin> sg.V(0l)
    ==>v[0]
    gremlin> sg.V(0l).bothE()
    ==>e[1][0-self->0]
    ==>e[1][0-self->0]
    gremlin> sg.V(0l).outE()
    ==>e[1][0-self->0]
    gremlin> sg.V(0l).inE()
    ==>e[1][0-self->0]
    gremlin>
    gremlin> // TADA!
    ==>true
    gremlin>
    ```
    
    `mvn clean install` and Spark integration tests.
    
    VOTE +1

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP-1284

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-tinkerpop/pull/298.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #298
    
----
commit 0563ec36bb823d617acde6a15217c2b61471408d
Author: Marko A. Rodriguez <ok...@gmail.com>
Date:   2016-04-29T15:33:29Z

    fixed a self-loop bug in StarGraph. Added StarGraphTest.shouldHandleSelfLoops() to ensure correct behavior.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP-1284: StarGraph does n...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-tinkerpop/pull/298


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP-1284: StarGraph does n...

Posted by dalaro <gi...@git.apache.org>.
Github user dalaro commented on a diff in the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/298#discussion_r61604599
  
    --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java ---
    @@ -290,7 +288,16 @@ public void dropVertexProperties(final String... propertyKeys) {
     
             @Override
             public Edge addEdge(final String label, final Vertex inVertex, final Object... keyValues) {
    -            return this.addOutEdge(label, inVertex, keyValues);
    +            final Edge edge = this.addOutEdge(label, inVertex, keyValues);
    +            if (inVertex.equals(this)) {
    +                List<Edge> inE = inEdges.get(label);
    --- End diff --
    
    inEdges may be null here on a pristine StarVertex


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP-1284: StarGraph does n...

Posted by dalaro <gi...@git.apache.org>.
Github user dalaro commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/298#issuecomment-215815666
  
    +1 (non-binding)
    
    This change fixed my formerly-broken StarGraph self-edge use case.
    
    I tested this PR's changes against 3.2 instead of 3.1.  Specifically, I checked out  e7b3267c99fe62cee0e887ca2b2d9aee17381631 and did `git cherry-pick -x apache/master..apache/TINKERPOP-1284`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP-1284: StarGraph does n...

Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/298#issuecomment-215830697
  
    `docker/build.sh -t -i -n` succeeded.
    
    VOTE: +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP-1284: StarGraph does n...

Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/298#issuecomment-216246229
  
    This is a good fix. PR says it is targeted for master but it is really for tp31. 
    
    VOTE +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---