Added support for or(has(x),has(y)) to be has(x.or(y)) in InlineFilterStrategy. While doing this, I found a bug in ConnectiveP steps where nested equivalents were not being inlined. That bug has been fixed. Added test cases to PTest to demonstrate proper inlining and nesting of ConnectivePs. I left two TODOs. One regarding match()-pulls that are labeled (interfering with FilterRankStrategy) and one regarding, has(x).has(y) being turned into has(x.and(y)). The reason why the latter isn't done now as it may greatly mess up providers who just rely on eq() for index lookups and are not smart enough to look into and()/or() predicates.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/8eb0fdde Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/8eb0fdde Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/8eb0fdde Branch: refs/heads/TINKERPOP-1458 Commit: 8eb0fdded4fd8943a1541fda33e0785ea10751ee Parents: 233a6ba Author: Marko A. Rodriguez <[email protected]> Authored: Thu Sep 29 10:39:20 2016 -0600 Committer: Marko A. Rodriguez <[email protected]> Committed: Mon Oct 3 08:24:17 2016 -0600 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 3 +- .../upgrade/release-3.2.x-incubating.asciidoc | 18 ++ .../optimization/InlineFilterStrategy.java | 45 +++- .../gremlin/process/traversal/util/AndP.java | 8 +- .../process/traversal/util/ConnectiveP.java | 3 +- .../gremlin/process/traversal/util/OrP.java | 9 +- .../gremlin/process/traversal/PTest.java | 245 ++++++++++--------- .../optimization/InlineFilterStrategyTest.java | 18 ++ .../TinkerGraphStepStrategyTest.java | 3 +- 9 files changed, 233 insertions(+), 119 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ade43cd..d204b44 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.2.3 (Release Date: NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Fixed a bug in `ConnectiveP` where nested equivalent connectives should be inlined. * Fixed a bug in `IncidentToAdjacentStrategy` where `TreeStep` traversals were allowed. * Fixed a end-step label bug in `MatchPredicateStrategy`. * Fixed a bug in `MatchPredicateStrategy` where inlined traversals did not have strategies applied to it. @@ -33,7 +34,7 @@ TinkerPop 3.2.3 (Release Date: NOT OFFICIALLY RELEASED YET) * Added `TraversalHelper.copyLabels()` for copying (or moving) labels form one step to another. * Added `TraversalHelper.applySingleLevelStrategies()` which will apply a subset of strategies but not walk the child tree. * Added the concept that hidden labels using during traversal compilation are removed at the end during `StandardVerificationStrategy`. (*breaking*) -* Added `InlineFilterStrategy` which will determine if a `TraversalFilterStep`, `AndStep`, `MatchStep` children are filters and if so, inline them. +* Added `InlineFilterStrategy` which will determine if a `TraversalFilterStep`, `AndStep`, `OrStep`, `MatchStep` children are filters and if so, inline them. * Removed `IdentityRemovalStrategy` from the default listing as its not worth the clock cycles. * Removed the "!" symbol in `NotStep.toString()` as it is confusing and the `NotStep`-name is sufficient. * Fixed a bug in `TraversalVertexProgram` (OLAP) around ordering and connectives (i.e. `and()` and `or()`). http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/docs/src/upgrade/release-3.2.x-incubating.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc index 0ba09bd..49790af 100644 --- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc +++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc @@ -171,6 +171,15 @@ uses of these exception builders as they will be removed in the future. See: link:https://issues.apache.org/jira/browse/TINKERPOP-944[TINKERPOP-944] +Hidden Step Labels for Compilation Only ++++++++++++++++++++++++++++++++++++++++ + +In order for `SubgraphStrategy` to work, it was necessary to have multi-level children communicate with one another +via hidden step labels. It was decided that hidden step labels are for compilation purposes only and will be removed +prior to traversal evaluation. This is a valid decision given that hidden labels for graph system providers are +not allowed to be used by users. Likewise, hidden labels for steps should not be allowed be used by +users as well. + PropertyMapStep with Selection Traversal ++++++++++++++++++++++++++++++++++++++++ @@ -181,6 +190,15 @@ and if so, use that in their introspection for respective strategies. This model See: link:https://issues.apache.org/jira/browse/TINKERPOP-1456[TINKERPOP-1456], link:https://issues.apache.org/jira/browse/TINKERPOP-844[TINKERPOP-844] +ConnectiveP Nesting Inlined ++++++++++++++++++++++++++++ + +There was a bug in `ConnectiveP` (`AndP`/`OrP`), where `eq(1).and(eq(2).and(eq(3)))` was `OrP(eq(1),AndP(eq(2),eq(3)))` +instead of unnested/inlined as `OrP(eq(1),eq(2),eq(3))`. Likewise, for `AndP`. If a provider was leveraging `ConnectiveP` +predicates for their custom steps (e.g. graph- or vertex-centric index lookups), then they should be aware of the inlining +and can simplify any and/or-tree walking code in their respective `ProviderOptimizationStrategy`. + +See: link:https://issues.apache.org/jira/browse/TINKERPOP-1470[TINKERPOP-1470] TinkerPop 3.2.2 --------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java index 7452b3b..0b7a5cc 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategy.java @@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization; import org.apache.tinkerpop.gremlin.process.computer.traversal.strategy.optimization.GraphFilterStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.Step; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; @@ -29,10 +30,12 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DedupGlobalSte import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DropStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.OrStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.RangeGlobalStep; import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; @@ -89,6 +92,46 @@ public final class InlineFilterStrategy extends AbstractTraversalStrategy<Traver traversal.removeStep(step); } } + // or(has(x,y),has(x,z)) --> has(x,y.or(z)) + for (final OrStep<?> step : TraversalHelper.getStepsOfClass(OrStep.class, traversal)) { + boolean process = true; + String key = null; + P predicate = null; + final List<String> labels = new ArrayList<>(); + for (final Traversal.Admin<?, ?> childTraversal : step.getLocalChildren()) { + this.apply(childTraversal); // todo: this may be a bad idea, but I can't seem to find a test case to break it + for (final Step<?, ?> childStep : childTraversal.getSteps()) { + if (childStep instanceof HasStep) { + for (final HasContainer hasContainer : ((HasStep<?>) childStep).getHasContainers()) { + if (null == key) + key = hasContainer.getKey(); + else if (!hasContainer.getKey().equals(key)) { + process = false; + break; + } + predicate = null == predicate ? + hasContainer.getPredicate() : + predicate.or(hasContainer.getPredicate()); + } + labels.addAll(childStep.getLabels()); + } else { + process = false; + break; + } + } + if (!process) + break; + } + if (process) { + changed = true; + final HasStep hasStep = new HasStep<>(traversal, new HasContainer(key, predicate)); + TraversalHelper.replaceStep(step, hasStep, traversal); + TraversalHelper.copyLabels(step, hasStep, false); + for (final String label : labels) { + hasStep.addLabel(label); + } + } + } // and(x,y) --> x.y for (final AndStep<?> step : TraversalHelper.getStepsOfClass(AndStep.class, traversal)) { boolean process = true; @@ -139,6 +182,7 @@ public final class InlineFilterStrategy extends AbstractTraversalStrategy<Traver TraversalHelper.applySingleLevelStrategies(traversal, matchTraversal, InlineFilterStrategy.class); step.removeGlobalChild(matchTraversal); step.getPreviousStep().addLabel(startLabel); + // TODO: matchTraversal.getEndStep().addLabel(startLabel); (perhaps insert an identity so filter rank can push has()-left) if (null != endLabel) matchTraversal.getEndStep().addLabel(endLabel); TraversalHelper.insertTraversal((Step) step.getPreviousStep(), matchTraversal, traversal); } @@ -149,7 +193,6 @@ public final class InlineFilterStrategy extends AbstractTraversalStrategy<Traver } } } - } private static final String determineStartLabelForHasPullOut(final MatchStep<?, ?> matchStep) { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/AndP.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/AndP.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/AndP.java index 8a7784f..cb001ff 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/AndP.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/AndP.java @@ -34,6 +34,9 @@ public final class AndP<V> extends ConnectiveP<V> { public AndP(final List<P<V>> predicates) { super(predicates); + for (final P<V> p : predicates) { + this.and(p); + } this.biPredicate = new AndBiPredicate(this); } @@ -49,7 +52,10 @@ public final class AndP<V> extends ConnectiveP<V> { public P<V> and(final Predicate<? super V> predicate) { if (!(predicate instanceof P)) throw new IllegalArgumentException("Only P predicates can be and'd together"); - this.predicates.add((P<V>) predicate); // TODO: clone and add? + else if (predicate instanceof AndP) + this.predicates.addAll(((AndP) predicate).getPredicates()); + else + this.predicates.add((P<V>) predicate); return this; } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ConnectiveP.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ConnectiveP.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ConnectiveP.java index 70f008a..855fc06 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ConnectiveP.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ConnectiveP.java @@ -31,13 +31,12 @@ import java.util.function.Predicate; */ public abstract class ConnectiveP<V> extends P<V> { - protected List<P<V>> predicates; + protected List<P<V>> predicates = new ArrayList<>(); public ConnectiveP(final List<P<V>> predicates) { super(null, null); if (predicates.size() < 2) throw new IllegalArgumentException("The provided " + this.getClass().getSimpleName() + " array must have at least two arguments: " + predicates.size()); - this.predicates = new ArrayList<>(predicates); // to avoid Arrays.asList() and unmodifiable exceptions } @Deprecated http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/OrP.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/OrP.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/OrP.java index 8bc4387..f7363b4 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/OrP.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/OrP.java @@ -22,7 +22,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import java.io.Serializable; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.function.BiPredicate; @@ -35,6 +34,9 @@ public final class OrP<V> extends ConnectiveP<V> { public OrP(final List<P<V>> predicates) { super(predicates); + for (final P<V> p : predicates) { + this.or(p); + } this.biPredicate = new OrBiPredicate(this); } @@ -50,7 +52,10 @@ public final class OrP<V> extends ConnectiveP<V> { public P<V> or(final Predicate<? super V> predicate) { if (!(predicate instanceof P)) throw new IllegalArgumentException("Only P predicates can be or'd together"); - this.predicates.add((P<V>) predicate); // TODO: clone and add? + else if (predicate instanceof OrP) + this.predicates.addAll(((OrP) predicate).getPredicates()); + else + this.predicates.add((P<V>) predicate); return this; } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTest.java index 3810c96..6ec33cc 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTest.java @@ -19,8 +19,11 @@ package org.apache.tinkerpop.gremlin.process.traversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.process.traversal.util.AndP; +import org.apache.tinkerpop.gremlin.process.traversal.util.OrP; import org.junit.Before; import org.junit.Test; +import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -29,135 +32,157 @@ import java.util.Arrays; import java.util.Random; import java.util.function.Predicate; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; /** * @author Daniel Kuppitz (http://gremlin.guru) */ -@RunWith(Parameterized.class) +@RunWith(Enclosed.class) public class PTest { - @Parameterized.Parameters(name = "{0}.test({1}) = {2}") - public static Iterable<Object[]> data() { - return new ArrayList<>(Arrays.asList(new Object[][]{ - {P.eq(0), 0, true}, - {P.eq(0), 1, false}, - {P.neq(0), 0, false}, - {P.neq(0), 1, true}, - {P.gt(0), -1, false}, - {P.gt(0), 0, false}, - {P.gt(0), 1, true}, - {P.lt(0), -1, true}, - {P.lt(0), 0, false}, - {P.lt(0), 1, false}, - {P.gte(0), -1, false}, - {P.gte(0), 0, true}, - {P.gte(0), 1, true}, - {P.lte(0), -1, true}, - {P.lte(0), 0, true}, - {P.lte(0), 1, false}, - {P.between(1, 10), 0, false}, - {P.between(1, 10), 1, true}, - {P.between(1, 10), 9, true}, - {P.between(1, 10), 10, false}, - {P.inside(1, 10), 0, false}, - {P.inside(1, 10), 1, false}, - {P.inside(1, 10), 9, true}, - {P.inside(1, 10), 10, false}, - {P.outside(1, 10), 0, true}, - {P.outside(1, 10), 1, false}, - {P.outside(1, 10), 9, false}, - {P.outside(1, 10), 10, false}, - {P.within(1, 2, 3), 0, false}, - {P.within(1, 2, 3), 1, true}, - {P.within(1, 2, 3), 10, false}, - {P.within(Arrays.asList(1, 2, 3)), 0, false}, - {P.within(Arrays.asList(1, 2, 3)), 1, true}, - {P.within(Arrays.asList(1, 2, 3)), 10, false}, - {P.without(1, 2, 3), 0, true}, - {P.without(1, 2, 3), 1, false}, - {P.without(1, 2, 3), 10, true}, - {P.without(Arrays.asList(1, 2, 3)), 0, true}, - {P.without(Arrays.asList(1, 2, 3)), 1, false}, - {P.without(Arrays.asList(1, 2, 3)), 10, true}, - {P.between("m", "n").and(P.neq("marko")), "marko", false}, - {P.between("m", "n").and(P.neq("marko")), "matthias", true}, - {P.between("m", "n").or(P.eq("daniel")), "marko", true}, - {P.between("m", "n").or(P.eq("daniel")), "daniel", true}, - {P.between("m", "n").or(P.eq("daniel")), "stephen", false}, - })); - } + @RunWith(Parameterized.class) + public static class ParameterizedTest { + + @Parameterized.Parameters(name = "{0}.test({1}) = {2}") + public static Iterable<Object[]> data() { + return new ArrayList<>(Arrays.asList(new Object[][]{ + {P.eq(0), 0, true}, + {P.eq(0), 1, false}, + {P.neq(0), 0, false}, + {P.neq(0), 1, true}, + {P.gt(0), -1, false}, + {P.gt(0), 0, false}, + {P.gt(0), 1, true}, + {P.lt(0), -1, true}, + {P.lt(0), 0, false}, + {P.lt(0), 1, false}, + {P.gte(0), -1, false}, + {P.gte(0), 0, true}, + {P.gte(0), 1, true}, + {P.lte(0), -1, true}, + {P.lte(0), 0, true}, + {P.lte(0), 1, false}, + {P.between(1, 10), 0, false}, + {P.between(1, 10), 1, true}, + {P.between(1, 10), 9, true}, + {P.between(1, 10), 10, false}, + {P.inside(1, 10), 0, false}, + {P.inside(1, 10), 1, false}, + {P.inside(1, 10), 9, true}, + {P.inside(1, 10), 10, false}, + {P.outside(1, 10), 0, true}, + {P.outside(1, 10), 1, false}, + {P.outside(1, 10), 9, false}, + {P.outside(1, 10), 10, false}, + {P.within(1, 2, 3), 0, false}, + {P.within(1, 2, 3), 1, true}, + {P.within(1, 2, 3), 10, false}, + {P.within(Arrays.asList(1, 2, 3)), 0, false}, + {P.within(Arrays.asList(1, 2, 3)), 1, true}, + {P.within(Arrays.asList(1, 2, 3)), 10, false}, + {P.without(1, 2, 3), 0, true}, + {P.without(1, 2, 3), 1, false}, + {P.without(1, 2, 3), 10, true}, + {P.without(Arrays.asList(1, 2, 3)), 0, true}, + {P.without(Arrays.asList(1, 2, 3)), 1, false}, + {P.without(Arrays.asList(1, 2, 3)), 10, true}, + {P.between("m", "n").and(P.neq("marko")), "marko", false}, + {P.between("m", "n").and(P.neq("marko")), "matthias", true}, + {P.between("m", "n").or(P.eq("daniel")), "marko", true}, + {P.between("m", "n").or(P.eq("daniel")), "daniel", true}, + {P.between("m", "n").or(P.eq("daniel")), "stephen", false}, + })); + } - @Parameterized.Parameter(value = 0) - public P predicate; + @Parameterized.Parameter(value = 0) + public P predicate; - @Parameterized.Parameter(value = 1) - public Object value; + @Parameterized.Parameter(value = 1) + public Object value; - @Parameterized.Parameter(value = 2) - public boolean expected; + @Parameterized.Parameter(value = 2) + public boolean expected; - @Test - public void shouldTest() { - assertEquals(expected, predicate.test(value)); - assertEquals(!expected, predicate.clone().negate().test(value)); - assertEquals(!expected, P.not(predicate).test(value)); - } - - @Before - public void init() { - final Object pv = predicate.getValue(); - final Random r = new Random(); - assertNotNull(predicate.getBiPredicate()); - predicate.setValue(r.nextDouble()); - assertNotNull(predicate.getValue()); - predicate.setValue(pv); - assertEquals(pv, predicate.getValue()); - assertNotNull(predicate.hashCode()); - assertEquals(predicate, predicate.clone()); - assertNotEquals(__.identity(), predicate); - - boolean thrown = true; - try { - predicate.and(new CustomPredicate()); - thrown = false; - } catch (IllegalArgumentException ex) { - assertEquals("Only P predicates can be and'd together", ex.getMessage()); - } finally { - assertTrue(thrown); + @Test + public void shouldTest() { + assertEquals(expected, predicate.test(value)); + assertEquals(!expected, predicate.clone().negate().test(value)); + assertEquals(!expected, P.not(predicate).test(value)); } - thrown = true; - try { - predicate.or(new CustomPredicate()); - thrown = false; - } catch (IllegalArgumentException ex) { - assertEquals("Only P predicates can be or'd together", ex.getMessage()); - } finally { - assertTrue(thrown); + @Before + public void init() { + final Object pv = predicate.getValue(); + final Random r = new Random(); + assertNotNull(predicate.getBiPredicate()); + predicate.setValue(r.nextDouble()); + assertNotNull(predicate.getValue()); + predicate.setValue(pv); + assertEquals(pv, predicate.getValue()); + assertNotNull(predicate.hashCode()); + assertEquals(predicate, predicate.clone()); + assertNotEquals(__.identity(), predicate); + + boolean thrown = true; + try { + predicate.and(new CustomPredicate()); + thrown = false; + } catch (IllegalArgumentException ex) { + assertEquals("Only P predicates can be and'd together", ex.getMessage()); + } finally { + assertTrue(thrown); + } + + thrown = true; + try { + predicate.or(new CustomPredicate()); + thrown = false; + } catch (IllegalArgumentException ex) { + assertEquals("Only P predicates can be or'd together", ex.getMessage()); + } finally { + assertTrue(thrown); + } } - } - private class CustomPredicate implements Predicate { + private class CustomPredicate implements Predicate { - @Override - public boolean test(Object o) { - return false; - } + @Override + public boolean test(Object o) { + return false; + } - @Override - public Predicate and(Predicate other) { - return null; - } + @Override + public Predicate and(Predicate other) { + return null; + } - @Override - public Predicate negate() { - return null; + @Override + public Predicate negate() { + return null; + } + + @Override + public Predicate or(Predicate other) { + return null; + } } + } - @Override - public Predicate or(Predicate other) { - return null; + public static class ConnectiveTest { + + @Test + public void shouldComposeCorrectly() { + assertEquals(P.eq(1), P.eq(1)); + assertEquals(P.eq(1).and(P.eq(2)), new AndP<>(Arrays.asList(P.eq(1), P.eq(2)))); + assertEquals(P.eq(1).and(P.eq(2).and(P.eq(3))), new AndP<>(Arrays.asList(P.eq(1), P.eq(2), P.eq(3)))); + assertEquals(P.eq(1).and(P.eq(2).and(P.eq(3).and(P.eq(4)))), new AndP<>(Arrays.asList(P.eq(1), P.eq(2), P.eq(3), P.eq(4)))); + assertEquals(P.eq(1).or(P.eq(2).or(P.eq(3).or(P.eq(4)))), new OrP<>(Arrays.asList(P.eq(1), P.eq(2), P.eq(3), P.eq(4)))); + assertEquals(P.eq(1).or(P.eq(2).and(P.eq(3).or(P.eq(4)))), new OrP<>(Arrays.asList(P.eq(1), new AndP<>(Arrays.asList(P.eq(2), new OrP<>(Arrays.asList(P.eq(3), P.eq(4)))))))); + assertEquals(P.eq(1).and(P.eq(2).or(P.eq(3).and(P.eq(4)))), new AndP<>(Arrays.asList(P.eq(1), new OrP<>(Arrays.asList(P.eq(2), new AndP<>(Arrays.asList(P.eq(3), P.eq(4)))))))); + assertEquals(P.eq(1).and(P.eq(2).and(P.eq(3).or(P.eq(4)))), new AndP<>(Arrays.asList(P.eq(1), P.eq(2), new OrP<>(Arrays.asList(P.eq(3), P.eq(4)))))); } } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java index aeae593..9530c82 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/InlineFilterStrategyTest.java @@ -29,6 +29,9 @@ import org.junit.runners.Parameterized; import java.util.Arrays; +import static org.apache.tinkerpop.gremlin.process.traversal.P.eq; +import static org.apache.tinkerpop.gremlin.process.traversal.P.gt; +import static org.apache.tinkerpop.gremlin.process.traversal.P.lt; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.V; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.and; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.as; @@ -39,6 +42,7 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.limit; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.map; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.match; +import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.or; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.out; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.tail; import static org.junit.Assert.assertEquals; @@ -46,6 +50,8 @@ import static org.junit.Assert.assertEquals; /** * @author Marko A. Rodriguez (http://markorodriguez.com) */ + + @RunWith(Parameterized.class) public class InlineFilterStrategyTest { @@ -74,9 +80,21 @@ public class InlineFilterStrategyTest { {filter(has("age", P.gt(10)).as("a")), has("age", P.gt(10)).as("a")}, {filter(and(has("age", P.gt(10)).as("a"), has("name", "marko"))), has("age", P.gt(10)).as("a").has("name", "marko")}, // + {or(has("name", "marko"), has("age", 32)), or(has("name", "marko"), has("age", 32))}, + {or(has("name", "marko"), has("name", "bob")), has("name", eq("marko").or(eq("bob")))}, + {or(has("name", "marko"), has("name")), or(has("name", "marko"), has("name"))}, + {or(has("age", 10), and(has("age", gt(20)), has("age", lt(100)))), has("age", eq(10).or(gt(20).and(lt(100))))}, + {or(has("name", "marko"), filter(has("name", "bob"))), has("name", eq("marko").or(eq("bob")))}, + {or(has("name", "marko"), filter(or(filter(has("name", "bob")), has("name", "stephen")))), has("name", eq("marko").or(eq("bob").or(eq("stephen"))))}, + {or(has("name", "marko").as("a"), filter(or(filter(has("name", "bob")).as("b"), has("name", "stephen").as("c")))), has("name", eq("marko").or(eq("bob").or(eq("stephen")))).as("a", "b", "c")}, + // {and(has("age", P.gt(10)), filter(has("age", 22))), has("age", P.gt(10)).has("age", 22)}, + {and(has("age", P.gt(10)).as("a"), filter(has("age", 22).as("b")).as("c")).as("d"), has("age", P.gt(10)).as("a").has("age", 22).as("b", "c", "d")}, {and(has("age", P.gt(10)).as("a"), and(filter(has("age", 22).as("b")).as("c"), has("name", "marko").as("d"))), has("age", P.gt(10)).as("a").has("age", 22).as("b", "c").has("name", "marko").as("d")}, + {and(has("age", P.gt(10)).as("a"), and(has("name","stephen").as("b"), has("name", "marko").as("c")).as("d")).as("e"), has("age", P.gt(10)).as("a").has("name","stephen").as("b").has("name","marko").as("c","d","e")}, {and(has("age", P.gt(10)), and(out("knows"), has("name", "marko"))), has("age", P.gt(10)).and(out("knows"), has("name", "marko"))}, + {and(has("age", P.gt(20)), or(has("age", lt(10)), has("age", gt(100)))), has("age", gt(20)).has("age", lt(10).or(gt(100)))}, + {and(has("age", P.gt(20)).as("a"), or(has("age", lt(10)), has("age", gt(100)).as("b"))), has("age", gt(20)).as("a").has("age", lt(10).or(gt(100))).as("b")}, // {V().match(as("a").has("age", 10), as("a").filter(has("name")).as("b")), V().as("a").has("age", 10).match(as("a").has("name").as("b"))}, {match(as("a").has("age", 10), as("a").filter(has("name")).as("b")), match(as("a").has("age", 10), as("a").has("name").as("b"))}, http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/8eb0fdde/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java index d2bc0ea..9f97466 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerGraphStepStrategyTest.java @@ -48,7 +48,6 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.filter import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.not; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.properties; -import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.values; import static org.junit.Assert.assertEquals; /** @@ -111,7 +110,7 @@ public class TinkerGraphStepStrategyTest { {__.V().as("a").dedup().has("name", "marko").or(has("age"), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).as("a").or(has("age"), has("age", gt(32))).dedup(), Arrays.asList(InlineFilterStrategy.instance(), FilterRankingStrategy.instance())}, {__.V().as("a").dedup().has("name", "marko").or(has("age", 10), has("age", gt(32))).filter(has("name", "bob")).has("lang", "java"), - g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).as("a").or(has("age", 10), has("age", gt(32))).dedup(), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, + g_V("name", eq("marko"), "age", eq(10).or(gt(32)), "name", eq("bob"), "lang", eq("java")).as("a").dedup(), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()}, {__.V().has("name", "marko").or(not(has("age")), has("age", gt(32))).has("name", "bob").has("lang", "java"), g_V("name", eq("marko"), "name", eq("bob"), "lang", eq("java")).or(not(filter(properties("age"))), has("age", gt(32))), TraversalStrategies.GlobalCache.getStrategies(TinkerGraph.class).toList()} });
