This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch pbulkset in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit d21d9423f9ef00c3d590f3aeeb1dcf71f506505a Author: Stephen Mallette <[email protected]> AuthorDate: Thu Feb 19 13:45:23 2026 -0500 Preserve BulkSet in P Doesn't address all the problems here with performance but for now just trying to preserve BulkSet for within/without which can happen with: aggregate('x')...where(without('x')) CTR --- CHANGELOG.asciidoc | 1 + .../tinkerpop/gremlin/process/traversal/P.java | 21 ++++- .../gremlin/process/traversal/PBulkSetTest.java | 97 ++++++++++++++++++++++ 3 files changed, 116 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index c22e267e86..6af38483f9 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -25,6 +25,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima This release also includes changes from <<release-3-7-6, 3.7.6>>. +* Preserved `BulkSet` when given to 'P'. * Fixed bug in Gremlin orderability semantics for `OffsetDateTime`. * Fixed bug in Javascript translation for UUID. * Fixed bug in pre-repeat() `emit()/until()` where `emit()` and `until()` traversers weren't added to the results. diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java index 559682a56e..de91007972 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java @@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal; import org.apache.commons.collections.CollectionUtils; import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.BulkSet; import org.apache.tinkerpop.gremlin.process.traversal.util.AndP; import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; @@ -71,7 +72,12 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { protected P(final PBiPredicate<V, V> biPredicate, final Collection<V> literals, final Map<String, V> variables, final boolean isCollection) { this.biPredicate = biPredicate; this.variables.putAll(variables); - this.literals = new ArrayList<>(literals); + if (literals instanceof BulkSet) { + this.literals = new BulkSet<>(); + this.literals.addAll(literals); + } else { + this.literals = new ArrayList<>(literals); + } this.isCollection = isCollection; } @@ -89,7 +95,16 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { */ public V getValue() { if (isCollection) { - Collection<V> values = this.literals.stream().collect(Collectors.toList()); + if (this.variables.isEmpty()) return (V) this.literals; + + final Collection<V> values; + if (this.literals instanceof BulkSet) { + values = new BulkSet<>(); + values.addAll(this.literals); + } else { + values = new ArrayList<>(this.literals); + } + values.addAll(this.variables.values()); return (V) values; } else if (!this.literals.isEmpty()) { @@ -113,7 +128,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { } else if (value instanceof Collection) { isCollection = true; if (((Collection<?>) value).stream().anyMatch(v -> v instanceof GValue)) { - this.literals = new ArrayList<>(); + this.literals = (value instanceof BulkSet) ? new BulkSet<>() : new ArrayList<>(); for (Object v : ((Collection<?>) value)) { // Separate variables and literals if (v instanceof GValue) { diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PBulkSetTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PBulkSetTest.java new file mode 100644 index 0000000000..e22517a2cd --- /dev/null +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PBulkSetTest.java @@ -0,0 +1,97 @@ +package org.apache.tinkerpop.gremlin.process.traversal; + +import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.BulkSet; +import org.junit.Test; + +import java.util.Collection; +import java.util.Collections; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Test class for {@link P} with {@link BulkSet} literals ensure that if a {@link BulkSet} is used as a literal, + * it is preserved in the {@link P} object's value, and that the {@link P} object can handle {@link BulkSet} literals + * with {@link GValue} elements correctly. You get a {@link BulkSet} in {@link P} when you have some Gremlin like: + * {@code aggreate('x')...where(within('x'))}. + */ +public class PBulkSetTest { + + @Test + public void shouldPreserveBulkSetInGetValue() { + final BulkSet<String> bulkSet = new BulkSet<>(); + bulkSet.add("a", 2); + bulkSet.add("b", 1); + + final P p = P.within(bulkSet); + final Object value = p.getValue(); + + assertTrue("Value should be a BulkSet but was " + value.getClass().getName(), value instanceof BulkSet); + assertEquals(bulkSet, value); + assertEquals(3, ((BulkSet) value).size()); + assertEquals(2L, ((BulkSet) value).get("a")); + } + + @Test + public void shouldPreserveBulkSetInSetValueWithGValues() { + final BulkSet<Object> bulkSet = new BulkSet<>(); + bulkSet.add("a", 2); + bulkSet.add(GValue.of("x", "b"), 1); + + final P p = P.within(Collections.emptyList()); + p.setValue(bulkSet); + + final Object value = p.getValue(); + // Currently this will fail and return an ArrayList (or a List from the stream) + assertTrue("Value should be a BulkSet but was " + (value == null ? "null" : value.getClass().getName()), value instanceof BulkSet); + assertEquals(3, ((BulkSet) value).size()); + assertEquals(2L, ((BulkSet) value).get("a")); + } + + @Test + public void shouldCloneBulkSetInGetValueWithVariables() { + final BulkSet<Object> bulkSet = new BulkSet<>(); + bulkSet.add("a", 2); + + // Subclass to access protected constructor and merge literals and variables + final P p = new PSubWithVars(Compare.eq, bulkSet, Collections.singletonMap("var1", "b")); + + final Object value = p.getValue(); + assertTrue("Value should be a BulkSet but was " + (value == null ? "null" : value.getClass().getName()), value instanceof BulkSet); + assertEquals(3, ((BulkSet) value).size()); + assertEquals(2L, ((BulkSet) value).get("a")); + assertEquals(1L, ((BulkSet) value).get("b")); + + // Ensure original bulkSet was not modified (p.getValue() should have cloned) + assertEquals(2, bulkSet.size()); + assertEquals(2L, bulkSet.get("a")); + assertEquals(0L, bulkSet.get("b")); + } + + @Test + public void shouldPreserveBulkSetInProtectedConstructor() throws Exception { + final BulkSet<String> bulkSet = new BulkSet<>(); + bulkSet.add("a", 2); + + final P p = new PSub(Compare.eq, bulkSet); + + final Object value = p.getValue(); + assertTrue("Value should be a BulkSet but was " + (value == null ? "null" : value.getClass().getName()), value instanceof BulkSet); + assertEquals(2, ((BulkSet) value).size()); + assertEquals(2L, ((BulkSet) value).get("a")); + } + + private static class PSub extends P { + public PSub(final PBiPredicate biPredicate, final Collection literals) { + super(biPredicate, literals, Collections.emptyMap(), true); + } + } + + private static class PSubWithVars extends P { + public PSubWithVars(final PBiPredicate biPredicate, final Collection literals, final Map variables) { + super(biPredicate, literals, variables, true); + } + } +}
