diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java index 142c95620b..5077845aad 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java @@ -41,6 +41,7 @@ import org.apache.hugegraph.backend.query.Query; import org.apache.hugegraph.exception.NotSupportException; import org.apache.hugegraph.iterator.FilterIterator; +import org.apache.hugegraph.schema.IndexLabel; import org.apache.hugegraph.schema.PropertyKey; import org.apache.hugegraph.schema.SchemaLabel; import org.apache.hugegraph.structure.HugeElement; @@ -67,6 +68,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.filter.RangeGlobalStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.CountGlobalStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.MaxGlobalStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.MeanGlobalStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.MinGlobalStep; @@ -166,11 +168,23 @@ public static void trySetGraph(Step step, HugeGraph graph) { public static void extractHasContainer(HugeGraphStep newStep, Traversal.Admin traversal) { - Step step = newStep; - do { - step = step.getNextStep(); + Step step = newStep.getNextStep(); + while (step instanceof HasStep || step instanceof NoOpBarrierStep) { + Step nextStep = step.getNextStep(); if (step instanceof HasStep) { HasContainerHolder holder = (HasContainerHolder) step; + /* + * Range/neq predicates before match() may trigger a no-index + * query after MatchStep reorders filters. Keep known-indexed + * boolean predicates pushed down, and leave the rest for + * TinkerPop to evaluate. + */ + if (followedByMatchStep(step) && + hasUnusableMatchPredicate(newStep, holder)) { + extractUsableHasContainers(newStep, holder); + step = nextStep; + continue; + } for (HasContainer has : holder.getHasContainers()) { if (!GraphStep.processHasContainerIds(newStep, has)) { newStep.addHasContainer(has); @@ -179,7 +193,103 @@ public static void extractHasContainer(HugeGraphStep newStep, TraversalHelper.copyLabels(step, step.getPreviousStep(), false); traversal.removeStep(step); } - } while (step instanceof HasStep || step instanceof NoOpBarrierStep); + step = nextStep; + } + } + + private static boolean followedByMatchStep(Step step) { + Step next = step.getNextStep(); + while (next instanceof HasStep || next instanceof NoOpBarrierStep) { + next = next.getNextStep(); + } + return next instanceof MatchStep; + } + + private static boolean hasUnusableMatchPredicate(HugeGraphStep step, + HasContainerHolder holder) { + HugeGraph graph = tryGetGraph(step); + for (HasContainer has : holder.getHasContainers()) { + if (!hasMatchIndexSensitivePredicate(has)) { + continue; + } + if (graph == null || !hasUsableMatchIndex(graph, step, has)) { + return true; + } + } + return false; + } + + private static void extractUsableHasContainers(HugeGraphStep step, + HasContainerHolder holder) { + HugeGraph graph = tryGetGraph(step); + for (HasContainer has : holder.getHasContainers()) { + if (hasMatchIndexSensitivePredicate(has) && + (graph == null || !hasUsableMatchIndex(graph, step, has))) { + continue; + } + if (!GraphStep.processHasContainerIds(step, has)) { + step.addHasContainer(has); + } + } + } + + private static boolean hasMatchIndexSensitivePredicate(HasContainer has) { + List> predicates = new ArrayList<>(); + collectPredicates(predicates, ImmutableList.of(has.getPredicate())); + for (P pred : predicates) { + BiPredicate bp = pred.getBiPredicate(); + if (bp == Compare.neq || + bp == Compare.gt || bp == Compare.gte || + bp == Compare.lt || bp == Compare.lte) { + return true; + } + } + return false; + } + + private static boolean hasUsableMatchIndex(HugeGraph graph, + HugeGraphStep step, + HasContainer has) { + if (isSysProp(has.getKey())) { + return true; + } + + PropertyKey pkey = graph.propertyKey(has.getKey()); + if (pkey.dataType() != DataType.BOOLEAN) { + return false; + } + + Collection schemaLabels = step.returnsVertex() ? + graph.vertexLabels() : + graph.edgeLabels(); + boolean seen = false; + for (SchemaLabel schemaLabel : schemaLabels) { + if (!schemaLabel.properties().contains(pkey.id())) { + continue; + } + seen = true; + if (!hasBooleanIndex(graph, schemaLabel, pkey)) { + return false; + } + } + return seen; + } + + private static boolean hasBooleanIndex(HugeGraph graph, + SchemaLabel schemaLabel, + PropertyKey pkey) { + for (Id id : schemaLabel.indexLabels()) { + IndexLabel indexLabel = graph.indexLabel(id); + if (indexLabel.indexFields().size() != 1 || + !indexLabel.indexField().equals(pkey.id())) { + continue; + } + if (indexLabel.indexType().isSecondary() || + indexLabel.indexType().isUnique()) { + return true; + } + } + return false; } public static void extractHasContainer(HugeVertexStep newStep, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/CountStrategyCoreTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/CountStrategyCoreTest.java index fdfc9d2e44..b6a099bd75 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/CountStrategyCoreTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/CountStrategyCoreTest.java @@ -19,8 +19,13 @@ import org.apache.hugegraph.schema.SchemaManager; import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.traversal.optimize.HugeGraphStep; import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.Step; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.Vertex; import org.junit.Test; @@ -48,6 +53,55 @@ private void initGraph() { commitTx(); } + private void initMatchNoIndexSchema() { + SchemaManager schema = graph().schema(); + schema.propertyKey("vp2").asBoolean().create(); + schema.propertyKey("vp3").asLong().create(); + schema.propertyKey("vp4").asText().create(); + schema.vertexLabel("vl1").properties("vp2", "vp4") + .nullableKeys("vp2", "vp4").create(); + schema.vertexLabel("vl0").properties("vp3") + .nullableKeys("vp3").create(); + schema.edgeLabel("el1").link("vl1", "vl0").create(); + } + + private void initMatchNoIndexGraph() { + Vertex v1 = graph().addVertex(T.label, "vl1", "vp2", true, + "vp4", "foo"); + Vertex v2 = graph().addVertex(T.label, "vl1", "vp2", false, + "vp4", "J2O"); + Vertex v3 = graph().addVertex(T.label, "vl0", + "vp3", 4592737712018141719L); + Vertex v4 = graph().addVertex(T.label, "vl0", + "vp3", 4592737712018141717L); + + v1.addEdge("el1", v3); + v2.addEdge("el1", v4); + commitTx(); + } + + private static HugeGraphStep applyAndGetGraphStep( + GraphTraversal traversal) { + traversal.asAdmin().applyStrategies(); + return (HugeGraphStep) traversal.asAdmin().getStartStep(); + } + + private static boolean hasRemainingHasStep(GraphTraversal traversal, + String key) { + for (Step step : traversal.asAdmin().getSteps()) { + if (!(step instanceof HasContainerHolder)) { + continue; + } + HasContainerHolder holder = (HasContainerHolder) step; + for (HasContainer has : holder.getHasContainers()) { + if (key.equals(has.getKey())) { + return true; + } + } + } + return false; + } + @Test public void testWhereCountLtNegativeIsAlwaysFalse() { this.initSchema(); @@ -125,4 +179,76 @@ public void testWhereCountGteNegativeDoesNotBuildInvalidRange() { Assert.assertEquals(4L, count); } + + @Test + public void testMatchWithNoIndexConditionMatchesDirectTraversal() { + this.initMatchNoIndexSchema(); + this.initMatchNoIndexGraph(); + + long direct = graph().traversal().V() + .has("vp4", P.neq("J2O")) + .has("vl1", "vp2", P.gte(false)) + .has("vp2") + .has("vl0", "vp3", P.gt(4592737712018141718L)) + .out("el1") + .count().next(); + long viaMatch = graph().traversal().V() + .has("vp4", P.neq("J2O")) + .has("vl1", "vp2", P.gte(false)) + .match(__.as("start0") + .has("vp2") + .has("vl0", "vp3", + P.gt(4592737712018141718L)) + .repeat(__.out("el1")) + .times(1) + .as("m0")) + .select("m0").count().next(); + + Assert.assertEquals(0L, direct); + Assert.assertEquals(direct, viaMatch); + } + + @Test + public void testMatchWithIndexedRangeConditionStillExtractsHas() { + this.initMatchNoIndexSchema(); + graph().schema().indexLabel("vl1ByVp2").onV("vl1") + .by("vp2").secondary().create(); + this.initMatchNoIndexGraph(); + + GraphTraversal traversal = graph().traversal().V() + .has("vp2", P.lt(true)) + .match(__.as("s") + .has("vp2") + .as("m")) + .select("m") + .count(); + + HugeGraphStep graphStep = applyAndGetGraphStep(traversal); + Assert.assertEquals(1, graphStep.getHasContainers().size()); + Assert.assertEquals("vp2", graphStep.getHasContainers().get(0).getKey()); + Assert.assertEquals(1L, traversal.next()); + } + + @Test + public void testMatchWithNoIndexConditionKeepsExtractingNextHas() { + this.initMatchNoIndexSchema(); + graph().schema().indexLabel("vl1ByVp2").onV("vl1") + .by("vp2").secondary().create(); + this.initMatchNoIndexGraph(); + + GraphTraversal traversal = graph().traversal().V() + .has("vp4", P.neq("J2O")) + .has("vp2", true) + .match(__.as("s") + .has("vp2") + .as("m")) + .select("m") + .count(); + + HugeGraphStep graphStep = applyAndGetGraphStep(traversal); + Assert.assertEquals(1, graphStep.getHasContainers().size()); + Assert.assertEquals("vp2", graphStep.getHasContainers().get(0).getKey()); + Assert.assertTrue(hasRemainingHasStep(traversal, "vp4")); + Assert.assertEquals(1L, traversal.next()); + } }