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/08 16:51:05 UTC
tinkerpop git commit: TINKERPOP-1979 Fixed OLAP bug with math() step
Repository: tinkerpop
Updated Branches:
refs/heads/TINKERPOP-1979 [created] 9f8f3b61b
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/TINKERPOP-1979
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() {