You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2023/07/07 17:39:16 UTC

[tinkerpop] branch master updated (d9d8d76d1a -> b0e74afa4c)

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


    from d9d8d76d1a Merge branch '3.6-dev'
     add fed6725a61 TINKERPOP-2920 Fixed bug in SubgraphStrategy when key not present
     new b2fb2b2465 Merge branch 'TINKERPOP-2920' into 3.5-dev
     new 82a78fcf74 Merge branch '3.5-dev' into 3.6-dev
     new b0e74afa4c Merge branch '3.6-dev'

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 CHANGELOG.asciidoc                                 |  5 +++--
 .../process/traversal/lambda/ValueTraversal.java   |  7 ++++++-
 .../traversal/step/map/PropertyMapStep.java        |  8 ++++----
 .../optimization/ProductiveByStrategy.java         | 24 +++++++++++++++++-----
 .../structure/io/graphson/GraphSONModule.java      |  1 +
 .../decoration/SubgraphStrategyProcessTest.java    | 23 +++++++++++++++++++++
 6 files changed, 56 insertions(+), 12 deletions(-)


[tinkerpop] 03/03: Merge branch '3.6-dev'

Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit b0e74afa4c601d27805fab06800ab579f2303454
Merge: d9d8d76d1a 82a78fcf74
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Fri Jul 7 13:38:47 2023 -0400

    Merge branch '3.6-dev'

 CHANGELOG.asciidoc                                 |  5 +++--
 .../process/traversal/lambda/ValueTraversal.java   |  7 ++++++-
 .../traversal/step/map/PropertyMapStep.java        |  8 ++++----
 .../optimization/ProductiveByStrategy.java         | 24 +++++++++++++++++-----
 .../structure/io/graphson/GraphSONModule.java      |  1 +
 .../decoration/SubgraphStrategyProcessTest.java    | 23 +++++++++++++++++++++
 6 files changed, 56 insertions(+), 12 deletions(-)



[tinkerpop] 01/03: Merge branch 'TINKERPOP-2920' into 3.5-dev

Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit b2fb2b246562ade571d88563e6dec10e7cbf3f37
Merge: 1cca37e512 fed6725a61
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Fri Jul 7 12:21:38 2023 -0400

    Merge branch 'TINKERPOP-2920' into 3.5-dev

 CHANGELOG.asciidoc                                 |  7 ++++---
 .../process/traversal/lambda/ValueTraversal.java   |  7 ++++++-
 .../traversal/step/map/PropertyMapStep.java        |  4 ++++
 .../optimization/ProductiveByStrategy.java         | 20 ++++++++++++++++--
 .../structure/io/graphson/GraphSONModule.java      |  1 +
 .../decoration/SubgraphStrategyProcessTest.java    | 24 ++++++++++++++++++++++
 6 files changed, 57 insertions(+), 6 deletions(-)

diff --cc CHANGELOG.asciidoc
index 6225ef9b79,0784fd4e1e..07f4ecb222
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@@ -24,14 -24,12 +24,15 @@@ image::https://raw.githubusercontent.co
  === TinkerPop 3.5.7 (Release Date: NOT OFFICIALLY RELEASED YET)
  
  * Fixed a memory leak in the Gremlin.Net driver that only occurred if a CancellationToken was provided.
 +* Fixed gremlin-python `Client` problem where calling `submit()` after` `close()` would hang the system.
  * Added `gremlin.spark.dontDeleteNonEmptyOutput` to stop deleting the output folder if it is not empty in `spark-gremlin`.
+ * Fixed a bug in `SubgraphStrategy` where the vertex property filter produced errors if a `Vertex` was missing the key provided to `by()` as a token.
  * Upgraded `gremlin-javascript` and `gremlint` to Node 16.20.0.
  * Upgraded `gremlin-go` to Go 1.20.
- * Improved the python Translator class
+ * Improved the python `Translator` class with better handling for `P`, `None` and subclasses of `str`.
  * Added `gremlin-java8.bat` file as a workaround to allow loading the console using Java 8 on Windows.
 +* Fixed a bug in `gremlin-server` where timeout tasks were not cancelled and could cause very large memory usage when timeout is large.
 +* Removed `jcabi-manifests` dependency from `gremlin-core`, `gremlin-driver`, and `gremlin-server`.
  
  [[release-3-5-6]]
  === TinkerPop 3.5.6 (Release Date: May 1, 2023)


[tinkerpop] 02/03: Merge branch '3.5-dev' into 3.6-dev

Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit 82a78fcf744ad4dc971ab7817ffd233fc00206fb
Merge: b355b6d002 b2fb2b2465
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Fri Jul 7 13:37:56 2023 -0400

    Merge branch '3.5-dev' into 3.6-dev

 CHANGELOG.asciidoc                                 |  5 +++--
 .../process/traversal/lambda/ValueTraversal.java   |  7 ++++++-
 .../traversal/step/map/PropertyMapStep.java        |  8 ++++----
 .../optimization/ProductiveByStrategy.java         | 24 +++++++++++++++++-----
 .../structure/io/graphson/GraphSONModule.java      |  1 +
 .../decoration/SubgraphStrategyProcessTest.java    | 23 +++++++++++++++++++++
 6 files changed, 56 insertions(+), 12 deletions(-)

diff --cc CHANGELOG.asciidoc
index 58ec3f2452,07f4ecb222..1a0eb72529
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@@ -359,12 -23,13 +359,13 @@@ image::https://raw.githubusercontent.co
  [[release-3-5-7]]
  === TinkerPop 3.5.7 (Release Date: NOT OFFICIALLY RELEASED YET)
  
 -* Fixed a memory leak in the Gremlin.Net driver that only occurred if a CancellationToken was provided.
 +* Fixed a memory leak in the Gremlin.Net driver that only occurred if a `CancellationToken` was provided.
  * Fixed gremlin-python `Client` problem where calling `submit()` after` `close()` would hang the system.
  * Added `gremlin.spark.dontDeleteNonEmptyOutput` to stop deleting the output folder if it is not empty in `spark-gremlin`.
+ * Fixed a bug in `SubgraphStrategy` where the vertex property filter produced errors if a `Vertex` was missing the key provided to `by()` as a token.
  * Upgraded `gremlin-javascript` and `gremlint` to Node 16.20.0.
  * Upgraded `gremlin-go` to Go 1.20.
- * Improved the python `Translator` class.
+ * Improved the python `Translator` class with better handling for `P`, `None` and subclasses of `str`.
  * Added `gremlin-java8.bat` file as a workaround to allow loading the console using Java 8 on Windows.
  * Fixed a bug in `gremlin-server` where timeout tasks were not cancelled and could cause very large memory usage when timeout is large.
  * Removed `jcabi-manifests` dependency from `gremlin-core`, `gremlin-driver`, and `gremlin-server`.
diff --cc gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
index f089636572,16d21935e7..812f13b4d2
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
@@@ -180,83 -211,7 +184,79 @@@ public class PropertyMapStep<K,E> exten
          return this.tokens;
      }
  
 -    private boolean includeToken(final int token) {
 +    protected boolean includeToken(final int token) {
          return 0 != (this.tokens & token);
      }
 +
 +    protected void addIncludedOptions(Element element, Map<Object, Object> map){
 +        if (this.returnType == PropertyType.VALUE) {
 +            if (includeToken(WithOptions.ids)) map.put(T.id, getElementId(element));
 +            if (element instanceof VertexProperty) {
 +
 +                if (includeToken(WithOptions.keys)) map.put(T.key, getVertexPropertyKey((VertexProperty<?>) element));
 +                if (includeToken(WithOptions.values)) map.put(T.value, getVertexPropertyValue((VertexProperty<?>) element));
 +            } else {
 +                if (includeToken(WithOptions.labels)) map.put(T.label, getElementLabel(element));
 +            }
 +        }
 +    }
 +
 +    protected Object getElementId(Element element){
 +        return element.id();
 +    }
 +
 +    protected String getElementLabel(Element element){
 +        return element.label();
 +    }
 +
 +    protected String getVertexPropertyKey(VertexProperty<?> vertexProperty){
 +        return vertexProperty.key();
 +    }
 +
 +    protected Object getVertexPropertyValue(VertexProperty<?> vertexProperty){
 +        return vertexProperty.value();
 +    }
 +
 +    protected void addElementProperties(final Traverser.Admin<Element> traverser, Map<Object, Object> map){
 +        final Element element = traverser.get();
 +        final boolean isVertex = element instanceof Vertex;
 +
 +        final Iterator<? extends Property> properties = null == this.propertyTraversal ?
 +                element.properties(this.propertyKeys) :
 +                TraversalUtil.applyAll(traverser, this.propertyTraversal);
 +
 +        while (properties.hasNext()) {
 +            final Property<?> property = properties.next();
 +            final Object value = this.returnType == PropertyType.VALUE ? property.value() : property;
 +            if (isVertex) {
 +                map.compute(property.key(), (k, v) -> {
 +                    final List<Object> values = v != null ? (List<Object>) v : new ArrayList<>();
 +                    values.add(value);
 +                    return values;
 +                });
 +            } else {
 +                map.put(property.key(), value);
 +            }
 +        }
 +    }
 +
 +    protected void applyTraversalRingToMap(Map<Object, Object> map){
 +        if (!traversalRing.isEmpty()) {
 +            // will cop a ConcurrentModification if a key is dropped so need this little copy here
 +            final List<Object> keys = new ArrayList<>(map.keySet());
 +            for (final Object key : keys) {
 +                map.compute(key, (k, v) -> {
 +                    final TraversalProduct product = TraversalUtil.produce(v, (Traversal.Admin) this.traversalRing.next());
 +
 +                    // compute() should take the null and remove the key
 +                    return product.isProductive() ? product.get() : null;
 +                });
 +            }
 +            this.traversalRing.reset();
 +        }
 +    }
 +
-     public Traversal.Admin<Element, ? extends Property> getPropertyTraversal() {
-         return propertyTraversal;
-     }
- 
 +    public TraversalRing<K, E> getTraversalRing() {
 +        return traversalRing;
 +    }
  }
diff --cc gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ProductiveByStrategy.java
index ba7b92d3c3,29c7bee647..ccdb820bce
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ProductiveByStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ProductiveByStrategy.java
@@@ -21,14 -21,14 +21,12 @@@ package org.apache.tinkerpop.gremlin.pr
  
  import org.apache.commons.configuration2.Configuration;
  import org.apache.commons.configuration2.MapConfiguration;
--import org.apache.tinkerpop.gremlin.process.traversal.Step;
  import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
  import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
  import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.DefaultGraphTraversal;
  import org.apache.tinkerpop.gremlin.process.traversal.lambda.ConstantTraversal;
  import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
  import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating;
--import org.apache.tinkerpop.gremlin.process.traversal.step.Grouping;
  import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
  import org.apache.tinkerpop.gremlin.process.traversal.step.map.CoalesceStep;
  import org.apache.tinkerpop.gremlin.process.traversal.step.map.ConstantStep;
@@@ -91,26 -92,51 +90,41 @@@ public class ProductiveByStrategy exten
          TraversalHelper.getStepsOfAssignableClass(ByModulating.class, traversal).stream().
                  filter(bm -> bm instanceof TraversalParent).forEach(bm -> {
              final TraversalParent parentStep = (TraversalParent) bm;
-             parentStep.getLocalChildren().forEach(child -> {
-                 if (child instanceof ValueTraversal && !containsValidByPass((ValueTraversal) child) &&
 -            final boolean parentStepIsGrouping = parentStep instanceof Grouping;
+             for (Traversal.Admin child : parentStep.getLocalChildren()) {
+                 // just outright skip the PropertyMapStep.propertyTraversal - it is only set by SubgraphStrategy and
+                 // for this case it doesn't make sense to force that traversal to be productive. if it is forced then
+                 // PropertyMap actually gets a null Property object if the traversal is not productive and that
+                 // causes an NPE. It doesn't make sense to catch the NPE there really as the code wants an empty
+                 // Iterator<Property> rather than a productive null. Actually, the propertyTraversal isn't even a
 -                // by() provided Traversal so it really doesn't make sense for this strategy to touch it. Another
++                // by() provided Traversal, so it really doesn't make sense for this strategy to touch it. Another
+                 // sort of example where strategies shouldn't have to know so much about one another. also, another
+                 // scenario where ProductiveByStrategy isn't so good. the default behavior without this strategy
+                 // works so much more nicely
+                 if (parentStep instanceof PropertyMapStep) {
+                     final Traversal.Admin propertyTraversal = ((PropertyMapStep) parentStep).getPropertyTraversal();
+                     if (propertyTraversal != null && propertyTraversal.equals(child))
+                         continue;
+                 }
+ 
 -                // Grouping requires a bit of special handling because of TINKERPOP-2627 which is a bug that has
 -                // some unexpected behavior for coalesce() as the value traversal. Grouping also has so inconsistencies
 -                // in how null vs filter behavior works and that behavior needs to stay to not break in 3.5.x
 -                if (!parentStepIsGrouping) {
 -                    if (child instanceof ValueTraversal && hasKeyNotKnownAsProductive((ValueTraversal) child)) {
 -                        wrapValueTraversalInCoalesce(parentStep, child);
 -                    } else if (!(child.getEndStep() instanceof ReducingBarrierStep)) {
 -                        // ending reducing barrier will always return something so seems safe to not bother wrapping
 -                        // that up in coalesce().
 -                        final Traversal.Admin extractedChildTraversal = new DefaultGraphTraversal<>();
 -                        TraversalHelper.removeToTraversal(child.getStartStep(), EmptyStep.instance(), extractedChildTraversal);
 -                        child.addStep(new CoalesceStep(child, extractedChildTraversal, nullTraversal));
++                if (child instanceof ValueTraversal &&  !containsValidByPass((ValueTraversal) child) &&
 +                        hasKeyNotKnownAsProductive((ValueTraversal) child)) {
 +                    wrapValueTraversalInCoalesce(parentStep, child);
 +                } else if (!(child.getEndStep() instanceof ReducingBarrierStep)) {
 +                    // ending reducing barrier will always return something so seems safe to not bother wrapping
 +                    // that up in coalesce().
 +                    final Traversal.Admin extractedChildTraversal = new DefaultGraphTraversal<>();
 +                    TraversalHelper.removeToTraversal(child.getStartStep(), EmptyStep.instance(), extractedChildTraversal);
 +                    child.addStep(new CoalesceStep(child, extractedChildTraversal, nullTraversal));
  
 -                        // replace so that internally the parent step gets to re-initialize the child as it may need to.
 -                        try {
 -                            parentStep.replaceLocalChild(child, child);
 -                        } catch (IllegalStateException ignored) {
 -                            // ignore situations where the parent traversal doesn't support replacement. in those cases
 -                            // we simply retain whatever the original behavior was even if it is inconsistent
 -                        }
 -                    }
 -                } else {
 -                    if (child instanceof ValueTraversal && ((Grouping) parentStep).getKeyTraversal() == child &&
 -                            hasKeyNotKnownAsProductive((ValueTraversal) child) && !containsValidByPass((ValueTraversal) child)) {
 -                        wrapValueTraversalInCoalesce(parentStep, child);
 +                    // replace so that internally the parent step gets to re-initialize the child as it may need to.
 +                    try {
 +                        parentStep.replaceLocalChild(child, child);
 +                    } catch (IllegalStateException ignored) {
 +                        // ignore situations where the parent traversal doesn't support replacement. in those cases
 +                        // we simply retain whatever the original behavior was even if it is inconsistent
                      }
                  }
-             });
+             }
          });
      }
  
diff --cc gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java
index 7bb4fa9cda,a378170931..ef26a080d2
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategyProcessTest.java
@@@ -508,6 -514,24 +514,23 @@@ public class SubgraphStrategyProcessTes
          checkResults(Arrays.asList(3, 3, 3, 4, 4, 5, 5, 5), sg.V().outE("uses").values("skill"));
          checkResults(Arrays.asList(3, 3, 3, 4, 4, 5, 5, 5), sg.V().as("a").properties().select("a").dedup().outE().values("skill"));
          checkResults(Arrays.asList(3, 3, 3, 4, 4, 5, 5, 5), sg.V().as("a").properties().select("a").dedup().outE().properties("skill").as("b").identity().select("b").by(__.value()));
+         //
+ 
+         // testing situations where the key specified is not present for the vertices - TINKERPOP-2920 use of
+         // constant(true) here is sorta weird because a Traversal<?,VertexProperty> is expected, but not really
+         sg = g.withStrategies(SubgraphStrategy.create(new MapConfiguration(new HashMap<String, Object>() {{
+             put(SubgraphStrategy.VERTEX_PROPERTIES, constant(true));
+         }})));
+         final List<Map<String, Object>> l = sg.V().project("example").by("example").toList();
+         l.forEach(map -> {
 -            assertEquals(1, map.size());
 -            assertNull(map.get("example"));
++            assertEquals(0, map.size());
+         });
+         checkResults(Arrays.asList("aachen","baltimore","bremen","brussels","centreville","dulles","kaiserslautern",
+                         "oakland","purcellville","san diego","santa cruz","santa fe","seattle","spremberg"),
+                 sg.withoutStrategies(ProductiveByStrategy.class).V().valueMap("location").select(Column.values).unfold().unfold());
+         checkResults(Arrays.asList("aachen","baltimore","bremen","brussels","centreville","dulles","kaiserslautern",
+                 "oakland","purcellville","san diego","santa cruz","santa fe","seattle","spremberg"),
+                 sg.V().valueMap("location").select(Column.values).unfold().unfold());
      }
  
      @Test