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

PropertyMapStep should reuse PropertiesStep

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: 3.0.2-incubating
    • Fix Version/s: 3.2.3
    • Component/s: process
    • Labels:
      None

      Description

      Currently, there are two steps which retrieve properties: PropertiesStep and PropertyMapStep. For any vendor implementation wishing to optimize property retrieval this means twice the (redundant) work because each of the steps needs to be extended individually.

      However, the functionality is very similar. PropertyMapStep should be replaced by two steps: PropertiesStep which returns an iterator over properties and a MapAggregatorStep (not tied to the name) which consumes properties from the previous step and builds the map (either with property or values as the the map value) until the element of a property differs.

      This reuse of PropertiesStep would have increase code reuse inside TinkerPop and should not have a significant performance impact. Most importantly, it makes life a lot easier for implementors.

        Activity

        Hide
        mbroecheler Matthias Broecheler added a comment -

        This would be incredibly useful to have for implementations like Titan that use a custom query optimizer to speed up traversals.

        Show
        mbroecheler Matthias Broecheler added a comment - This would be incredibly useful to have for implementations like Titan that use a custom query optimizer to speed up traversals.
        Hide
        okram Marko A. Rodriguez added a comment - - edited

        Its interesting as we can do this now:

        gremlin> g.V().propertyMap()
        ==>[name:[vp[name->marko]], age:[vp[age->29]]]
        ==>[name:[vp[name->vadas]], age:[vp[age->27]]]
        ==>[name:[vp[name->lop]], lang:[vp[lang->java]]]
        ==>[name:[vp[name->josh]], age:[vp[age->32]]]
        ==>[name:[vp[name->ripple]], lang:[vp[lang->java]]]
        ==>[name:[vp[name->peter]], age:[vp[age->35]]]
        gremlin> g.V().map(properties().group().by(key).by())
        ==>[name:[vp[name->marko]], age:[vp[age->29]]]
        ==>[name:[vp[name->vadas]], age:[vp[age->27]]]
        ==>[name:[vp[name->lop]], lang:[vp[lang->java]]]
        ==>[name:[vp[name->josh]], age:[vp[age->32]]]
        ==>[name:[vp[name->ripple]], lang:[vp[lang->java]]]
        ==>[name:[vp[name->peter]], age:[vp[age->35]]]
        

        However, check the problem – 5x plus speed difference:

        gremlin> clock(10000){g.V().propertyMap().next()}
        ==>0.014044303999999999
        gremlin> clock(10000){g.V().map(properties().group().by(key).by()).next()}
        ==>0.0804077743
        

        Also, there is the problem of:

        gremlin> g.V().valueMap(true)
        ==>[name:[marko], id:1, label:person, age:[29]]
        ==>[name:[vadas], id:2, label:person, age:[27]]
        ==>[name:[lop], id:3, label:software, lang:[java]]
        ==>[name:[josh], id:4, label:person, age:[32]]
        ==>[name:[ripple], id:5, label:software, lang:[java]]
        ==>[name:[peter], id:6, label:person, age:[35]]
        gremlin> g.V().map(properties().group().by(key).by(value))
        ==>[name:[marko], age:[29]]
        ==>[name:[vadas], age:[27]]
        ==>[name:[lop], lang:[java]]
        ==>[name:[josh], age:[32]]
        ==>[name:[ripple], lang:[java]]
        ==>[name:[peter], age:[35]]
        

        How do we solve the true parameter where we need the id and label. ........ :/ Well, we can make a wrapping step, etc... but its ugly.

        Show
        okram Marko A. Rodriguez added a comment - - edited Its interesting as we can do this now: gremlin> g.V().propertyMap() ==>[name:[vp[name->marko]], age:[vp[age->29]]] ==>[name:[vp[name->vadas]], age:[vp[age->27]]] ==>[name:[vp[name->lop]], lang:[vp[lang->java]]] ==>[name:[vp[name->josh]], age:[vp[age->32]]] ==>[name:[vp[name->ripple]], lang:[vp[lang->java]]] ==>[name:[vp[name->peter]], age:[vp[age->35]]] gremlin> g.V().map(properties().group().by(key).by()) ==>[name:[vp[name->marko]], age:[vp[age->29]]] ==>[name:[vp[name->vadas]], age:[vp[age->27]]] ==>[name:[vp[name->lop]], lang:[vp[lang->java]]] ==>[name:[vp[name->josh]], age:[vp[age->32]]] ==>[name:[vp[name->ripple]], lang:[vp[lang->java]]] ==>[name:[vp[name->peter]], age:[vp[age->35]]] However, check the problem – 5x plus speed difference: gremlin> clock(10000){g.V().propertyMap().next()} ==>0.014044303999999999 gremlin> clock(10000){g.V().map(properties().group().by(key).by()).next()} ==>0.0804077743 Also, there is the problem of: gremlin> g.V().valueMap( true ) ==>[name:[marko], id:1, label:person, age:[29]] ==>[name:[vadas], id:2, label:person, age:[27]] ==>[name:[lop], id:3, label:software, lang:[java]] ==>[name:[josh], id:4, label:person, age:[32]] ==>[name:[ripple], id:5, label:software, lang:[java]] ==>[name:[peter], id:6, label:person, age:[35]] gremlin> g.V().map(properties().group().by(key).by(value)) ==>[name:[marko], age:[29]] ==>[name:[vadas], age:[27]] ==>[name:[lop], lang:[java]] ==>[name:[josh], age:[32]] ==>[name:[ripple], lang:[java]] ==>[name:[peter], age:[35]] How do we solve the true parameter where we need the id and label. ........ :/ Well, we can make a wrapping step, etc... but its ugly.
        Hide
        mhfrantz Matt Frantz added a comment -

        How about an overload of properties(final boolean includeTokens, String...propertyKeys)? You'd have to synthesize Property objects to hold the ID and label, including a string key, which for these purposes could be 'id' and 'label' to align with valueMap. Is that what you mean by "wrapping step"?

        Show
        mhfrantz Matt Frantz added a comment - How about an overload of properties(final boolean includeTokens, String...propertyKeys) ? You'd have to synthesize Property objects to hold the ID and label, including a string key, which for these purposes could be 'id' and 'label' to align with valueMap . Is that what you mean by "wrapping step"?
        Hide
        okram Marko A. Rodriguez added a comment -

        @dkuppitz – thoughts here would be great.

        Show
        okram Marko A. Rodriguez added a comment - @dkuppitz – thoughts here would be great.
        Hide
        okram Marko A. Rodriguez added a comment -

        Please see PropertyMapStep. In master/ (3.2.3-SNAPSHOT) a PropertyMapStep can now take a child traversal that gets desired properties. The old model of simply using vertex.properties(propertyKeys) still exists. However, as a provider, you can setPropertyTraversal() to __.properties(propertyKeys) to get the desired effect.

        https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java

        If this is sufficient, please close this ticket, else provide feedback.

        Show
        okram Marko A. Rodriguez added a comment - Please see PropertyMapStep . In master/ (3.2.3-SNAPSHOT) a PropertyMapStep can now take a child traversal that gets desired properties. The old model of simply using vertex.properties(propertyKeys) still exists. However, as a provider, you can setPropertyTraversal() to __.properties(propertyKeys) to get the desired effect. https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java If this is sufficient, please close this ticket, else provide feedback.
        Hide
        spmallette stephen mallette added a comment -

        Looks like this was completed a while back - there was not additional feedback so I will assume it is done to satisfaction.

        Show
        spmallette stephen mallette added a comment - Looks like this was completed a while back - there was not additional feedback so I will assume it is done to satisfaction.

          People

          • Assignee:
            okram Marko A. Rodriguez
            Reporter:
            mbroecheler Matthias Broecheler
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development