spmallette commented on code in PR #3458:
URL: https://github.com/apache/tinkerpop/pull/3458#discussion_r3439672531


##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java:
##########
@@ -116,21 +147,50 @@ public V getValue() {
     }
 
     public void setValue(final V value) {
-        variables.clear();
-        literals = Collections.EMPTY_LIST;
+        // Full reset — completely replace the predicate's value state.
+        this.childTraversals = null;
+        this.variables.clear();
+        this.literals = Collections.EMPTY_LIST;
+        this.isCollection = false;
+        this.resolvedEmpty = false;
+        applyValue(value);
+    }
 
-        if (value == null) {
-            isCollection = false;
+    /**
+     * Core value-processing logic. Interprets the given value and populates 
the appropriate internal
+     * fields ({@code childTraversals}, {@code literals}, {@code variables}, 
{@code isCollection}).
+     * Does NOT reset state first — the caller is responsible for ensuring a 
clean slate if needed.
+     */
+    @SuppressWarnings("unchecked")
+    private void applyValue(final V value) {
+        if (value instanceof GraphTraversal) {
+            final Traversal.Admin<?, ?> admin = ((Traversal<?, ?>) 
value).asAdmin();
+            ChildTraversalValidator.validate(admin);
+            this.childTraversals = Collections.singletonList(admin);
+        } else if (value == null) {
             this.literals = Collections.singleton(null);
         } else if (value instanceof GValue) {
             variables.put(((GValue<V>) value).getName(), ((GValue<V>) 
value).get());
-            isCollection = false;
         } else if (value instanceof Collection) {
-            isCollection = true;
-            if (((Collection<?>) value).stream().anyMatch(v -> v instanceof 
GValue)) {
+            final Collection<?> coll = (Collection<?>) value;
+            if (!coll.isEmpty() && coll.stream().anyMatch(v -> v instanceof 
GraphTraversal)) {
+                if (!coll.stream().allMatch(v -> v instanceof GraphTraversal)) 
{
+                    throw new IllegalArgumentException(
+                            "Cannot mix traversals and literal values. " +
+                            "Use __.constant(val) to wrap all values as 
traversals.");

Review Comment:
   > "Use __.constant(val) to wrap all values as traversals."
   
   maybe better as:
   
   > "Use __.constant(val) to wrap each value as a traversals."
   
   that's more clear, no?



-- 
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]

Reply via email to