Cole-Greer commented on code in PR #3157:
URL: https://github.com/apache/tinkerpop/pull/3157#discussion_r2268436786
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java:
##########
@@ -37,10 +43,27 @@ public class P<V> implements Predicate<V>, Serializable,
Cloneable {
protected PBiPredicate<V, V> biPredicate;
protected V value;
protected V originalValue;
+ protected GValue<V> gValue;
+ protected GValue<V>[] gValues;
+
+ public P(final PBiPredicate<V, V> biPredicate, V value) {
+ if (value instanceof GValue) {
+ this.gValue = (GValue<V>) value;
+ this.value = ((GValue<V>) value).get();
+ } else if (value instanceof List && ((List) value).stream().anyMatch(v
-> v instanceof GValue)) {
Review Comment:
Retaining the original `Collection` ran into more trouble than I
anticipated. This PR has `P` maintain parallel lists for both the GValue and
scalar forms of `value`. When `updateVariable()` is called, the index of a
matched GValue is used to determine which value in the scalar list also needs
updating. This approach falls apart if we want to store non-indexed collections.
This, along with some fragility concerns regarding the parallel arrays
approach led me to consider introducing a `PInterface` and `GValueP` which can
be reduced to a regular `P` similarly to how the step placeholders work. I've
built that out to be about 95% right and pushed it to a [separate
branch](https://github.com/apache/tinkerpop/tree/GValue-3.8-PInterface),
although I'm not sure it's the right approach as it brings so much more change.
I'm going to put a pin in this for the moment and focus on refinements
elsewhere in this PR, and I will come back and reconsider the P solution
afterwards.
--
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]
