Cole-Greer commented on code in PR #3209: URL: https://github.com/apache/tinkerpop/pull/3209#discussion_r2373550949
########## gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractMergeElementStepPlaceholder.java: ########## Review Comment: In the context of mergeV/E, we currently never call `integrateChild` for the `properties`, and we we also never return any of the properties from `getLocalChildren`. The mergeV/E steps do not really consider the properties to be child traversals. The properties really only exist in mergeV/E to support `PartitionStrategy`, and it's arguably not the right way to do this. PartitionStrategy only ever stores String values in the properties too so there really should never be a Traversal here in practice. The proper solution here is to introduce a dedicated interface such as `Partitioning` to support `PartitionStrategy`, and then get properties out of mergeV/E. I consider this out of scope for this PR, and for the meantime I'm happy with the current state here where although Traversal's may theoretically be possible in properties, they are not considered child traversals of this step. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
