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() {