TINKERPOP-1878 Added a test for ordering and corrected some problems in logic
Ordering didn't work - at least not completely. The test, as it is written in this commit, that failed to sort properly. Changed the logic for ordering to track all of the keys specified to ORDER with their appropriate ASC/DESC operators and added all of them to the traversal. Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/114b2097 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/114b2097 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/114b2097 Branch: refs/heads/TINKERPOP-1878 Commit: 114b20976491f4b642f70cdd55a6d0a263f42023 Parents: 7c3c6f6 Author: Stephen Mallette <[email protected]> Authored: Fri Jan 26 15:41:10 2018 -0500 Committer: Stephen Mallette <[email protected]> Committed: Mon Apr 23 14:21:04 2018 -0400 ---------------------------------------------------------------------- .../sparql/SparqlToGremlinTranspiler.java | 55 ++++++-------------- .../dsl/sparql/SparqlTraversalSourceTest.java | 31 +++++++++-- 2 files changed, 44 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/114b2097/sparql-gremlin/src/main/java/org/apache/tinkerpop/gremlin/sparql/SparqlToGremlinTranspiler.java ---------------------------------------------------------------------- diff --git a/sparql-gremlin/src/main/java/org/apache/tinkerpop/gremlin/sparql/SparqlToGremlinTranspiler.java b/sparql-gremlin/src/main/java/org/apache/tinkerpop/gremlin/sparql/SparqlToGremlinTranspiler.java index 9db7d82..ea3f828 100644 --- a/sparql-gremlin/src/main/java/org/apache/tinkerpop/gremlin/sparql/SparqlToGremlinTranspiler.java +++ b/sparql-gremlin/src/main/java/org/apache/tinkerpop/gremlin/sparql/SparqlToGremlinTranspiler.java @@ -21,9 +21,10 @@ package org.apache.tinkerpop.gremlin.sparql; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; -import org.apache.jena.graph.Triple; import org.apache.jena.query.Query; import org.apache.jena.query.QueryFactory; import org.apache.jena.query.SortCondition; @@ -58,8 +59,6 @@ public class SparqlToGremlinTranspiler { private List<Traversal> traversalList = new ArrayList<>(); - private String sortingVariable = ""; - private SparqlToGremlinTranspiler(final GraphTraversal<Vertex, ?> traversal) { this.traversal = traversal; } @@ -96,31 +95,18 @@ public class SparqlToGremlinTranspiler { final int numberOfTraversal = traversalList.size(); final Traversal arrayOfAllTraversals[] = new Traversal[numberOfTraversal]; - if (query.hasOrderBy() && !query.hasGroupBy()) { - final List<SortCondition> sortingConditions = query.getOrderBy(); - - for (SortCondition sortCondition : sortingConditions) { - final Expr expr = sortCondition.getExpression(); - sortingVariable = expr.getVarName(); - } - } - for (Traversal tempTrav : traversalList) { arrayOfAllTraversals[traversalIndex++] = tempTrav; } - Order orderDirection = Order.incr; + final Map<String, Order> orderingIndex = new HashMap<>(); if (query.hasOrderBy()) { - int directionOfSort = 0; final List<SortCondition> sortingConditions = query.getOrderBy(); for (SortCondition sortCondition : sortingConditions) { final Expr expr = sortCondition.getExpression(); - directionOfSort = sortCondition.getDirection(); - sortingVariable = expr.getVarName(); + orderingIndex.put(expr.getVarName(), sortCondition.getDirection() == 1 ? Order.incr : Order.decr); } - - if (directionOfSort == -1) orderDirection = Order.decr; } if (traversalList.size() > 0) @@ -128,6 +114,8 @@ public class SparqlToGremlinTranspiler { final List<String> vars = query.getResultVars(); if (!query.isQueryResultStar() && !query.hasGroupBy()) { + // the result sizes have special handling to get the right signatures of select() called. perhaps this + // could be refactored to work more nicely switch (vars.size()) { case 0: throw new IllegalStateException(); @@ -135,20 +123,16 @@ public class SparqlToGremlinTranspiler { if (query.isDistinct()) traversal = traversal.dedup(vars.get(0)); - if (query.hasOrderBy()) - traversal = traversal.order().by(sortingVariable, orderDirection); - else - traversal = traversal.select(vars.get(0)); + orderingIndex.forEach((k,v) -> traversal = traversal.order().by(__.select(k), v)); + traversal = traversal.select(vars.get(0)); break; case 2: if (query.isDistinct()) traversal = traversal.dedup(vars.get(0), vars.get(1)); - if (query.hasOrderBy()) - traversal = traversal.order().by(__.select(vars.get(0)), orderDirection).by(__.select(vars.get(1))); - else - traversal = traversal.select(vars.get(0), vars.get(1)); + orderingIndex.forEach((k,v) -> traversal = traversal.order().by(__.select(k), v)); + traversal = traversal.select(vars.get(0), vars.get(1)); break; default: @@ -157,11 +141,11 @@ public class SparqlToGremlinTranspiler { if (query.isDistinct()) traversal = traversal.dedup(all); + orderingIndex.forEach((k,v) -> traversal = traversal.order().by(__.select(k), v)); + + // just some shenanigans to get the right signature of select() called final String[] others = Arrays.copyOfRange(all, 2, vars.size()); - if (query.hasOrderBy()) - traversal = traversal.order().by(__.select(vars.get(0)), orderDirection).by(__.select(vars.get(1))); - else - traversal = traversal.select(vars.get(0), vars.get(1), others); + traversal = traversal.select(vars.get(0), vars.get(1), others); break; } @@ -202,7 +186,7 @@ public class SparqlToGremlinTranspiler { } if (query.hasOrderBy() && query.hasGroupBy()) - traversal = traversal.order().by(sortingVariable, orderDirection); + orderingIndex.forEach((k,v) -> traversal = traversal.order().by(__.select(k), v)); if (query.hasLimit()) { long limit = query.getLimit(), offset = 0; @@ -233,14 +217,7 @@ public class SparqlToGremlinTranspiler { */ @Override public void visit(final OpBGP opBGP) { - final List<Triple> triples = opBGP.getPattern().getList(); - final Traversal[] matchTraversals = new Traversal[triples.size()]; - int i = 0; - for (final Triple triple : triples) { - - matchTraversals[i++] = TraversalBuilder.transform(triple); - traversalList.add(matchTraversals[i - 1]); - } + opBGP.getPattern().getList().forEach(triple -> traversalList.add(TraversalBuilder.transform(triple))); } /** http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/114b2097/sparql-gremlin/src/test/java/org/apache/tinkerpop/gremlin/sparql/process/traversal/dsl/sparql/SparqlTraversalSourceTest.java ---------------------------------------------------------------------- diff --git a/sparql-gremlin/src/test/java/org/apache/tinkerpop/gremlin/sparql/process/traversal/dsl/sparql/SparqlTraversalSourceTest.java b/sparql-gremlin/src/test/java/org/apache/tinkerpop/gremlin/sparql/process/traversal/dsl/sparql/SparqlTraversalSourceTest.java index 9bb6025..2743255 100644 --- a/sparql-gremlin/src/test/java/org/apache/tinkerpop/gremlin/sparql/process/traversal/dsl/sparql/SparqlTraversalSourceTest.java +++ b/sparql-gremlin/src/test/java/org/apache/tinkerpop/gremlin/sparql/process/traversal/dsl/sparql/SparqlTraversalSourceTest.java @@ -27,16 +27,18 @@ import java.util.List; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder; +import static org.hamcrest.collection.IsIterableContainingInOrder.contains; /** * @author Stephen Mallette (http://stephen.genoprime.com) */ public class SparqlTraversalSourceTest { + private static final Graph graph = TinkerFactory.createModern(); + private static final SparqlTraversalSource g = graph.traversal(SparqlTraversalSource.class); + @Test - public void shouldDoStuff() { - final Graph graph = TinkerFactory.createModern(); - final SparqlTraversalSource g = graph.traversal(SparqlTraversalSource.class); + public void shouldFindAllPersonsNamesAndAges() { final List<?> x = g.sparql("SELECT ?name ?age WHERE { ?person v:name ?name . ?person v:age ?age }").toList(); assertThat(x, containsInAnyOrder( new HashMap<String,Object>(){{ @@ -57,4 +59,27 @@ public class SparqlTraversalSourceTest { }} )); } + + @Test + public void shouldFindAllPersonsNamesAndAgesOrdered() { + final List<?> x = g.sparql("SELECT ?name ?age WHERE { ?person v:name ?name . ?person v:age ?age } ORDER BY ASC(?age)").toList(); + assertThat(x, contains( + new HashMap<String,Object>(){{ + put("name", "vadas"); + put("age", 27); + }}, + new HashMap<String,Object>(){{ + put("name", "marko"); + put("age", 29); + }}, + new HashMap<String,Object>(){{ + put("name", "josh"); + put("age", 32); + }}, + new HashMap<String,Object>(){{ + put("name", "peter"); + put("age", 35); + }} + )); + } }
