caught another bug in FilterRankStrategy. We really need to get the max rank of the traversal parent's nested steps. Effects AndStep, OrStep, WhereTraversalStep, WherePredicateStep, DedupStep, OrderStep. This ensures that filter(where().by()) isn't pushed forward cause its wrapped in filter().
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/997e94fb Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/997e94fb Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/997e94fb Branch: refs/heads/TINKERPOP-1502 Commit: 997e94fb489afc10cc9bcfc6c92106f021ca1045 Parents: 226633f Author: Marko A. Rodriguez <okramma...@gmail.com> Authored: Tue Nov 15 13:46:44 2016 -0700 Committer: Marko A. Rodriguez <okramma...@gmail.com> Committed: Wed Nov 16 05:44:18 2016 -0700 ---------------------------------------------------------------------- .../optimization/FilterRankingStrategy.java | 41 ++++++++++++++------ .../optimization/FilterRankingStrategyTest.java | 4 +- 2 files changed, 33 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/997e94fb/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java index 15040f7..3649615 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategy.java @@ -120,32 +120,51 @@ public final class FilterRankingStrategy extends AbstractTraversalStrategy<Trave * @return The rank of the given step. */ private static int getStepRank(final Step step) { + final int rank; if (!(step instanceof FilterStep || step instanceof OrderGlobalStep)) return 0; else if (step instanceof IsStep || step instanceof ClassFilterStep) - return 1; + rank = 1; else if (step instanceof HasStep) - return 2; + rank = 2; else if (step instanceof WherePredicateStep && ((WherePredicateStep) step).getLocalChildren().isEmpty()) - return 3; + rank = 3; else if (step instanceof SimplePathStep || step instanceof CyclicPathStep) - return 4; + rank = 4; else if (step instanceof TraversalFilterStep || step instanceof NotStep) - return 5; + rank = 5; else if (step instanceof WhereTraversalStep) - return 6; + rank = 6; else if (step instanceof OrStep) - return 7; + rank = 7; else if (step instanceof AndStep) - return 8; + rank = 8; else if (step instanceof WherePredicateStep) // has by()-modulation - return 9; + rank = 9; else if (step instanceof DedupGlobalStep) - return 10; + rank = 10; else if (step instanceof OrderGlobalStep) - return 11; + rank = 11; else return 0; + //////////// + if (step instanceof TraversalParent) + return getMaxStepRank((TraversalParent) step, rank); + else + return rank; + } + + private static int getMaxStepRank(final TraversalParent parent, final int startRank) { + int maxStepRank = startRank; + // no filter steps are global parents (yet) + for (final Traversal.Admin<?, ?> traversal : parent.getLocalChildren()) { + for (final Step<?, ?> step : traversal.getSteps()) { + final int stepRank = getStepRank(step); + if (stepRank > maxStepRank) + maxStepRank = stepRank; + } + } + return maxStepRank; } private static boolean usesLabels(final Step<?, ?> step, final Set<String> labels) { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/997e94fb/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java index b32ae2a..11f390b 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/FilterRankingStrategyTest.java @@ -112,10 +112,12 @@ public class FilterRankingStrategyTest { {__.as("a").out().and(has("age"), has("name")).where(P.eq("a")), __.as("a").out().where(P.eq("a")).and(has("age"), has("name")), Collections.emptyList()}, {__.as("a").out().and(has("age"), has("name")).where(P.eq("a")).by("age"), __.as("a").out().has("age").has("name").where(P.eq("a")).by("age"), Collections.singletonList(InlineFilterStrategy.instance())}, {__.as("a").out().and(has("age"), has("name")).where(P.eq("a")), __.as("a").out().where(P.eq("a")).has("age").has("name"), Collections.singletonList(InlineFilterStrategy.instance())}, + {__.as("a").out().and(has("age"), has("name")).filter(__.where(P.eq("a"))), __.as("a").out().where(P.eq("a")).has("age").has("name"), Collections.singletonList(InlineFilterStrategy.instance())}, + {__.as("a").out().and(has("age"), has("name")).filter(__.where(P.eq("a")).by("age")), __.as("a").out().has("age").has("name").where(P.eq("a")).by("age"), Collections.singletonList(InlineFilterStrategy.instance())}, {has("value", 0).filter(out()).dedup(), has("value", 0).filter(out()).dedup(), Collections.emptyList()}, {has("value", 0).or(has("name"), has("age")).has("value", 1).dedup(), has("value", 0).has("value", 1).or(has("name"), has("age")).dedup(), Collections.singletonList(InlineFilterStrategy.instance())}, {has("value", 0).or(out(), in()).as(Graph.Hidden.hide("x")).has("value", 1).dedup(), has("value", 0).has("value", 1).or(outE(), inE()).dedup(), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, - // {has("value", 0).and(has("age"), has("name", "marko")).is(10), __.is(10).has("value", 0).has("name", "marko").has("age"), Collections.singletonList(InlineFilterStrategy.instance())}, + {has("value", 0).and(has("age"), has("name", "marko")).is(10), __.is(10).has("value", 0).has("age").has("name", "marko"), Collections.singletonList(InlineFilterStrategy.instance())}, {has("value", 0).filter(or(not(has("age")), has("age", 1))).has("value", 1).dedup(), has("value", 0).has("value", 1).or(not(filter(properties("age"))), has("age", 1)).dedup(), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()}, }); }
