Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-333

Support VertexProperty in PartitionStrategy

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Implemented
    • Affects Version/s: 3.0.1-incubating
    • Fix Version/s: 3.1.0-incubating
    • Component/s: process
    • Labels:
      None

      Description

      Make it so that partitions extend to VertexProperty elements thus allowing properties on vertices to fall into different partitions.

        Activity

        Hide
        spmallette stephen mallette added a comment -

        I've worked around all the issues I formerly found, at least insofar as they pertain to PartitionStrategy), and have VertexProperty working with it. Additional work could be done to preserve optimizations related to cardinality settings for a call to GraphTraversal.property() that gets folded into a {AddVertex/Start/EdgeStep}}, but that can be considered some other time.

        Show
        spmallette stephen mallette added a comment - I've worked around all the issues I formerly found, at least insofar as they pertain to PartitionStrategy ), and have VertexProperty working with it. Additional work could be done to preserve optimizations related to cardinality settings for a call to GraphTraversal.property() that gets folded into a {AddVertex/Start/EdgeStep}}, but that can be considered some other time.
        Hide
        spmallette stephen mallette added a comment -

        Looks like PartitionStrategy can work if we don't do this folding:

        https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java#L990-L991

        That allows me to append the partition key meta properties to individual AddProperty steps. With the folding there into VertexStep and AddVertexStartStep you can't include the partition key because those steps don't support setting meta properties through them.

        Show
        spmallette stephen mallette added a comment - Looks like PartitionStrategy can work if we don't do this folding: https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java#L990-L991 That allows me to append the partition key meta properties to individual AddProperty steps. With the folding there into VertexStep and AddVertexStartStep you can't include the partition key because those steps don't support setting meta properties through them.
        Hide
        spmallette stephen mallette added a comment -

        I'm not so sure this works for SubgraphStrategy either at least not for what I pictured happening for subgraphing. I would think we are trying to exclude specific properties in the subgraph. So consider g.V() - where g has a SubgraphStrategy that hides ALL properties - the vertices that get returned from that still have all their properties. Don't think we can't stop that.......

        Show
        spmallette stephen mallette added a comment - I'm not so sure this works for SubgraphStrategy either at least not for what I pictured happening for subgraphing. I would think we are trying to exclude specific properties in the subgraph. So consider g.V() - where g has a SubgraphStrategy that hides ALL properties - the vertices that get returned from that still have all their properties. Don't think we can't stop that.......
        Hide
        spmallette stephen mallette added a comment -

        I don't think PartitionStrategy is going to work with VertexProperty. Not sure if that's a bad thing as far as use cases go (as PartitionStrategy was secondarily mentioned on the ticket - perhaps not)?

        When I do g.addV().property('name','stephen') i get to evaluate AddVertexStep in the strategy but have no way to tack in a partitionKey/Value onto that newly added "name" property.

        Going to take a look SubgraphStrategy.

        Show
        spmallette stephen mallette added a comment - I don't think PartitionStrategy is going to work with VertexProperty. Not sure if that's a bad thing as far as use cases go (as PartitionStrategy was secondarily mentioned on the ticket - perhaps not)? When I do g.addV().property('name','stephen') i get to evaluate AddVertexStep in the strategy but have no way to tack in a partitionKey/Value onto that newly added "name" property. Going to take a look SubgraphStrategy .
        Hide
        spmallette stephen mallette added a comment -

        Moved to 3.1.0 only since TINKERPOP3-783 seems largely resolved for that version.

        Show
        spmallette stephen mallette added a comment - Moved to 3.1.0 only since TINKERPOP3-783 seems largely resolved for that version.
        Hide
        spmallette stephen mallette added a comment -

        The above issue regarding asVertex is described in its own issue here: TINKERPOP3-783

        Show
        spmallette stephen mallette added a comment - The above issue regarding asVertex is described in its own issue here: TINKERPOP3-783
        Hide
        spmallette stephen mallette added a comment -

        Was just looking at this in greater detail - the asVertex member variable is set via:

        https://github.com/apache/incubator-tinkerpop/blob/be3aa546be934a0e3e1bba0f090af18f1f6ab22d/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java#L53

        essentially:

        this.asVertex = null != cardinality || this.vertexPropertyKeyValues.length > 0;
        

        but that doesn't will allow asVertex to be set to false if called as follows:

        __.outV().property("name", "stephen")
        

        which means it then isn't always reliable for determining if the property being set applies to a Vertex and therefore isn't good for determining if a partition key/value should be popped in as a meta-property.

        Show
        spmallette stephen mallette added a comment - Was just looking at this in greater detail - the asVertex member variable is set via: https://github.com/apache/incubator-tinkerpop/blob/be3aa546be934a0e3e1bba0f090af18f1f6ab22d/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java#L53 essentially: this .asVertex = null != cardinality || this .vertexPropertyKeyValues.length > 0; but that doesn't will allow asVertex to be set to false if called as follows: __.outV().property( "name" , "stephen" ) which means it then isn't always reliable for determining if the property being set applies to a Vertex and therefore isn't good for determining if a partition key/value should be popped in as a meta-property.
        Hide
        spmallette stephen mallette added a comment -

        no - this is not done. we decided we would do it post-GA.

        Show
        spmallette stephen mallette added a comment - no - this is not done. we decided we would do it post-GA.
        Hide
        okram Marko A. Rodriguez added a comment -

        stephen mallette Is this done? The problem was that VertexProperties were not under such "strategy filters" a long time ago (i.e. via their meta-properties).

        Show
        okram Marko A. Rodriguez added a comment - stephen mallette Is this done? The problem was that VertexProperties were not under such "strategy filters" a long time ago (i.e. via their meta-properties).
        Hide
        okram Marko A. Rodriguez added a comment -

        This needs to be handled for a big user.

        Show
        okram Marko A. Rodriguez added a comment - This needs to be handled for a big user.

          People

          • Assignee:
            spmallette stephen mallette
            Reporter:
            okram Marko A. Rodriguez
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development