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 2018/06/18 17:34:59 UTC

[1/3] tinkerpop git commit: TINKERPOP-1979 Fixed OLAP bug with math() step

Repository: tinkerpop
Updated Branches:
  refs/heads/master 2b319ab0c -> b21436999


TINKERPOP-1979 Fixed OLAP bug with math() step

math() can now execute in OLAP with by() modulators if the expression given to math() does not access path labels.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/9f8f3b61
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/9f8f3b61
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/9f8f3b61

Branch: refs/heads/master
Commit: 9f8f3b61b8e69624b2ac7aac3c98dcace55a079f
Parents: 55fcbdc
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Fri Jun 8 12:50:00 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Fri Jun 8 12:50:00 2018 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../process/traversal/step/map/MathStep.java    | 13 ++++++++
 .../process/traversal/step/map/MathTest.java    | 32 ++++++++++++++++++++
 3 files changed, 46 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9f8f3b61/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 199b689..edbb009 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 This release also includes changes from <<release-3-2-10, 3.2.10>>.
 
 * Deprecated `Order` for `incr` and `decr` in favor of `asc` and `desc`.
+* Fixed bug in `math()` for OLAP where `ComputerVerificationStrategy` was incorrectly detecting path label access and preventing execution.
 
 [[release-3-3-3]]
 === TinkerPop 3.3.3 (Release Date: May 8, 2018)

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9f8f3b61/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
index b34c3ca..e259eaf 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
@@ -86,6 +86,19 @@ public final class MathStep<S> extends MapStep<S, Double> implements ByModulatin
     }
 
     @Override
+    public ElementRequirement getMaxRequirement() {
+        // this is a trick i saw in DedupGlobalStep that allows ComputerVerificationStrategy to be happy for OLAP.
+        // it's a bit more of a hack here. in DedupGlobalStep, the dedup operation really only just needs the ID, but
+        // here the true max requirement is PROPERTIES, but because of how map() works in this implementation in
+        // relation to CURRENT, if we don't access the path labels, then we really only just operate on the stargraph
+        // and are thus OLAP safe. In tracing around the code a bit, I don't see a problem with taking this approach,
+        // but I suppose a better way might be make it more clear when this step is dealing with an actual path and
+        // when it is not and/or adjust ComputerVerificationStrategy to cope with the situation where math() is only
+        // dealing with the local stargraph.
+        return (variables.contains(CURRENT) && variables.size() == 1) ? ElementRequirement.ID : PathProcessor.super.getMaxRequirement();
+    }
+
+    @Override
     public String toString() {
         return StringFactory.stepString(this, this.equation, this.traversalRing);
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9f8f3b61/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathTest.java
index 04096ed..9753e7d 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathTest.java
@@ -38,6 +38,7 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.bothE;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.in;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.math;
 import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.sack;
+import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.select;
 import static org.junit.Assert.assertEquals;
 
 /**
@@ -46,6 +47,10 @@ import static org.junit.Assert.assertEquals;
 @RunWith(GremlinProcessRunner.class)
 public abstract class MathTest extends AbstractGremlinProcessTest {
 
+    public abstract Traversal<Vertex, Double> get_g_V_outE_mathX0_minus_itX_byXweightX();
+
+    public abstract Traversal<Vertex, Double> get_g_V_hasXageX_valueMap_mathXit_plus_itXbyXselectXageX_unfoldXX();
+
     public abstract Traversal<Vertex, Double> get_g_V_asXaX_outXknowsX_asXbX_mathXa_plus_bX_byXageX();
 
     public abstract Traversal<Vertex, Double> get_g_withSideEffectXx_100X_V_age_mathX__plus_xX();
@@ -58,6 +63,22 @@ public abstract class MathTest extends AbstractGremlinProcessTest {
 
     @Test
     @LoadGraphWith(MODERN)
+    public void g_V_outE_mathX0_minus_itX_byXweightX() {
+        final Traversal<Vertex, Double> traversal = get_g_V_outE_mathX0_minus_itX_byXweightX();
+        printTraversalForm(traversal);
+        checkResults(Arrays.asList(-0.4, -0.4, -0.5, -1.0, -1.0, -0.2), traversal);
+    }
+
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_V_hasXageX_valueMap_mathXit_plus_itXbyXselectXageX_unfoldXX() {
+        final Traversal<Vertex, Double> traversal = get_g_V_hasXageX_valueMap_mathXit_plus_itXbyXselectXageX_unfoldXX();
+        printTraversalForm(traversal);
+        checkResults(Arrays.asList(64.0, 58.0, 54.0, 70.0), traversal);
+    }
+
+    @Test
+    @LoadGraphWith(MODERN)
     public void g_V_asXaX_outXknowsX_asXbX_mathXa_plus_bX_byXageX() {
         final Traversal<Vertex, Double> traversal = get_g_V_asXaX_outXknowsX_asXbX_mathXa_plus_bX_byXageX();
         printTraversalForm(traversal);
@@ -101,6 +122,17 @@ public abstract class MathTest extends AbstractGremlinProcessTest {
     }
 
     public static class Traversals extends MathTest {
+        @Override
+        public Traversal<Vertex, Double> get_g_V_outE_mathX0_minus_itX_byXweightX() {
+            // https://issues.apache.org/jira/browse/TINKERPOP-1979 - should work in OLAP
+            return g.V().outE().math("0-_").by("weight");
+        }
+
+        @Override
+        public Traversal<Vertex, Double> get_g_V_hasXageX_valueMap_mathXit_plus_itXbyXselectXageX_unfoldXX() {
+            // https://issues.apache.org/jira/browse/TINKERPOP-1979 - should work in OLAP
+            return g.V().has("age").valueMap().math("_+_").by(select("age").unfold());
+        }
 
         @Override
         public Traversal<Vertex, Double> get_g_V_asXaX_outXknowsX_asXbX_mathXa_plus_bX_byXageX() {


[2/3] tinkerpop git commit: TINKERPOP-1979 Get math() working on spark properly

Posted by sp...@apache.org.
TINKERPOP-1979 Get math() working on spark properly

The Expression class was not serializable and Spark was not happy. Wrapped it up in another class that was and now tests work on spark ok.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/a6e0a2d5
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/a6e0a2d5
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/a6e0a2d5

Branch: refs/heads/master
Commit: a6e0a2d55c6b5bb9a710a53b8dfc208696416fe2
Parents: 9f8f3b6
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Fri Jun 8 16:53:30 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Fri Jun 8 16:53:30 2018 -0400

----------------------------------------------------------------------
 .../process/traversal/step/map/MathStep.java    | 53 +++++++++++++++-----
 1 file changed, 40 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/a6e0a2d5/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
index e259eaf..0994411 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MathStep.java
@@ -33,6 +33,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalRing;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
 
+import java.io.Serializable;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -47,19 +48,14 @@ public final class MathStep<S> extends MapStep<S, Double> implements ByModulatin
 
     private static final String CURRENT = "_";
     private final String equation;
-    private final Set<String> variables;
-    private final Expression expression;
+    private final TinkerExpression expression;
     private TraversalRing<S, Number> traversalRing = new TraversalRing<>();
     private Set<String> keepLabels;
 
     public MathStep(final Traversal.Admin traversal, final String equation) {
         super(traversal);
         this.equation = equation;
-        this.variables = MathStep.getVariables(this.equation);
-        this.expression = new ExpressionBuilder(this.equation)
-                .variables(this.variables)
-                .implicitMultiplication(false)
-                .build();
+        this.expression = new TinkerExpression(equation, MathStep.getVariables(this.equation));
     }
 
     @Override
@@ -69,8 +65,8 @@ public final class MathStep<S> extends MapStep<S, Double> implements ByModulatin
 
     @Override
     protected Double map(final Traverser.Admin<S> traverser) {
-        final Expression localExpression = new Expression(this.expression);
-        for (final String var : this.variables) {
+        final Expression localExpression = new Expression(this.expression.getExpression());
+        for (final String var : this.expression.getVariables()) {
             localExpression.setVariable(var,
                     var.equals(CURRENT) ?
                             TraversalUtil.applyNullable(traverser, this.traversalRing.next()).doubleValue() :
@@ -95,7 +91,8 @@ public final class MathStep<S> extends MapStep<S, Double> implements ByModulatin
         // but I suppose a better way might be make it more clear when this step is dealing with an actual path and
         // when it is not and/or adjust ComputerVerificationStrategy to cope with the situation where math() is only
         // dealing with the local stargraph.
-        return (variables.contains(CURRENT) && variables.size() == 1) ? ElementRequirement.ID : PathProcessor.super.getMaxRequirement();
+        return (this.expression.getVariables().contains(CURRENT) && this.expression.getVariables().size() == 1) ?
+                ElementRequirement.ID : PathProcessor.super.getMaxRequirement();
     }
 
     @Override
@@ -139,12 +136,12 @@ public final class MathStep<S> extends MapStep<S, Double> implements ByModulatin
 
     @Override
     public Set<String> getScopeKeys() {
-        if (this.variables.contains(CURRENT)) {
-            final Set<String> temp = new HashSet<>(this.variables);
+        if (this.expression.getVariables().contains(CURRENT)) {
+            final Set<String> temp = new HashSet<>(this.expression.getVariables());
             temp.remove(CURRENT);
             return temp;
         } else
-            return this.variables;
+            return this.expression.getVariables();
     }
 
     @Override
@@ -181,4 +178,34 @@ public final class MathStep<S> extends MapStep<S, Double> implements ByModulatin
         return variables;
     }
 
+    /**
+     * A wrapper for the {@code Expression} class. That class is not marked {@code Serializable} and therefore gives
+     * problems in OLAP specifically with Spark. This wrapper allows the {@code Expression} to be serialized in that
+     * context with Java serialization.
+     */
+    public static class TinkerExpression implements Serializable {
+        private transient Expression expression;
+        private final String equation;
+        private final Set<String> variables;
+
+        public TinkerExpression(final String equation, final Set<String> variables) {
+            this.variables = variables;
+            this.equation = equation;
+        }
+
+        public Expression getExpression() {
+            if (null == expression) {
+                this.expression = new ExpressionBuilder(this.equation)
+                        .variables(this.variables)
+                        .implicitMultiplication(false)
+                        .build();
+            }
+            return expression;
+        }
+
+        public Set<String> getVariables() {
+            return variables;
+        }
+    }
+
 }


[3/3] tinkerpop git commit: Merge branch 'TINKERPOP-1979'

Posted by sp...@apache.org.
Merge branch 'TINKERPOP-1979'


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/b2143699
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/b2143699
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/b2143699

Branch: refs/heads/master
Commit: b21436999b1ce003a789f54485f4353b4cfb4b0d
Parents: 2b319ab a6e0a2d
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Mon Jun 18 13:34:39 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Mon Jun 18 13:34:39 2018 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  1 +
 .../process/traversal/step/map/MathStep.java    | 64 ++++++++++++++++----
 .../process/traversal/step/map/MathTest.java    | 32 ++++++++++
 3 files changed, 85 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b2143699/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --cc CHANGELOG.asciidoc
index f7429fe,edbb009..d361518
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@@ -63,8 -25,8 +63,9 @@@ image::https://raw.githubusercontent.co
  
  This release also includes changes from <<release-3-2-10, 3.2.10>>.
  
 +* Removed `timedInterrupt` from documentation as a way to timeout.
  * Deprecated `Order` for `incr` and `decr` in favor of `asc` and `desc`.
+ * Fixed bug in `math()` for OLAP where `ComputerVerificationStrategy` was incorrectly detecting path label access and preventing execution.
  
  [[release-3-3-3]]
  === TinkerPop 3.3.3 (Release Date: May 8, 2018)