Cole-Greer commented on code in PR #3209:
URL: https://github.com/apache/tinkerpop/pull/3209#discussion_r2373401373
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddElementStepPlaceholder.java:
##########
@@ -175,6 +188,9 @@ public void addProperty(Object key, Object value) {
if (key instanceof GValue) {
throw new IllegalArgumentException("GValue cannot be used as a
property key");
}
+ if (key instanceof Traversal) {
+ this.integrateChild(((Traversal<?, ?>) key).asAdmin());
Review Comment:
Historically speaking, no. The main purpose of integrateChild is to ensure
the child is consistent with the parent (has the correct parent set, both
traversals merge their side effects, and now they also merge their
GValueManagers). Historically speaking there hasn't been much concern about the
state of the child after it has been removed, as it is no longer a concern for
the TraversalParent. One could argue that the removed child should be given an
EmptyTraversal as a new parent, however this has never been enforced in the
past.
--
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]