tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmalle...@apache.org
Subject [1/3] tinkerpop git commit: TINKERPOP-1979 Fixed OLAP bug with math() step
Date Mon, 18 Jun 2018 17:34:59 GMT
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 <spmva@genoprime.com>
Authored: Fri Jun 8 12:50:00 2018 -0400
Committer: Stephen Mallette <spmva@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()
{


Mime
View raw message