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/bb21a3df Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/bb21a3df Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/bb21a3df Branch: refs/heads/TINKERPOP-1878 Commit: bb21a3df342d620405085a5434a655a2b4b4a3cb Parents: e364a9d Author: Stephen Mallette <sp...@genoprime.com> Authored: Fri Jan 26 15:41:10 2018 -0500 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Wed Aug 8 07:47:30 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/bb21a3df/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/bb21a3df/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); + }} + )); + } }
