This is an automated email from the ASF dual-hosted git repository.
imbajin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hugegraph.git
The following commit(s) were added to refs/heads/master by this push:
new 03e6b8e9e fix(core): keep unsafe range filters local (#3068)
03e6b8e9e is described below
commit 03e6b8e9efbd62f1374fc926edef681ef07a1b4d
Author: legendpei <[email protected]>
AuthorDate: Mon Jun 29 12:56:27 2026 +0800
fix(core): keep unsafe range filters local (#3068)
---
.../hugegraph/auth/HugeFactoryAuthProxy.java | 6 +
.../main/java/org/apache/hugegraph/HugeGraph.java | 4 +-
.../optimize/HugeConnectiveLabelStepStrategy.java | 132 ++++++++++++
.../traversal/optimize/TraversalUtil.java | 234 +++++++++++++++++++--
.../hugegraph/core/CountStrategyCoreTest.java | 123 +++++++++++
.../optimize/TraversalUtilOptimizeTest.java | 222 +++++++++++++++++++
6 files changed, 703 insertions(+), 18 deletions(-)
diff --git
a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java
b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java
index 57486a407..b9cdb4fd6 100644
---
a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java
+++
b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java
@@ -54,6 +54,7 @@ import org.apache.hugegraph.task.StandardTaskScheduler;
import org.apache.hugegraph.task.TaskCallable;
import org.apache.hugegraph.task.TaskCallable.SysTaskCallable;
import org.apache.hugegraph.task.TaskManager;
+import org.apache.hugegraph.traversal.optimize.HugeConnectiveLabelStepStrategy;
import org.apache.hugegraph.traversal.optimize.HugeCountStepStrategy;
import org.apache.hugegraph.traversal.optimize.HugeGraphStepStrategy;
import org.apache.hugegraph.traversal.optimize.HugeVertexStepStrategy;
@@ -468,6 +469,10 @@ public final class HugeFactoryAuthProxy {
"createDefaultExecutor",
"lambda$start$0", "start");
Reflection.registerFieldsToFilter(JsonSerializer.class, "LBUF_SIZE",
"INSTANCE");
Reflection.registerMethodsToFilter(JsonSerializer.class,
"writeIterator", "instance");
+
Reflection.registerFieldsToFilter(HugeConnectiveLabelStepStrategy.class,
+ "serialVersionUID", "INSTANCE");
+
Reflection.registerMethodsToFilter(HugeConnectiveLabelStepStrategy.class,
+ "instance");
Reflection.registerFieldsToFilter(HugeVertexStepStrategy.class,
"serialVersionUID",
"INSTANCE");
Reflection.registerMethodsToFilter(HugeVertexStepStrategy.class,
"instance");
@@ -559,6 +564,7 @@ public final class HugeFactoryAuthProxy {
registerPrivateActions(LockManager.class);
registerPrivateActions(ServerReporter.class);
registerPrivateActions(JsonSerializer.class);
+ registerPrivateActions(HugeConnectiveLabelStepStrategy.class);
registerPrivateActions(HugeVertexStepStrategy.class);
registerPrivateActions(HugeGraphStepStrategy.class);
registerPrivateActions(HugeCountStepStrategy.class);
diff --git
a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java
b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java
index deaa458c2..8fcd76d39 100644
---
a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java
+++
b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java
@@ -49,6 +49,7 @@ import org.apache.hugegraph.schema.SchemaManager;
import org.apache.hugegraph.schema.VertexLabel;
import org.apache.hugegraph.structure.HugeFeatures;
import org.apache.hugegraph.task.TaskScheduler;
+import org.apache.hugegraph.traversal.optimize.HugeConnectiveLabelStepStrategy;
import org.apache.hugegraph.traversal.optimize.HugeCountStrategy;
import org.apache.hugegraph.traversal.optimize.HugeCountStepStrategy;
import org.apache.hugegraph.traversal.optimize.HugeGraphStepStrategy;
@@ -381,7 +382,8 @@ public interface HugeGraph extends Graph {
.getStrategies(Graph.class)
.clone();
strategies.removeStrategies(CountStrategy.class);
- strategies.addStrategies(HugeCountStrategy.instance(),
+ strategies.addStrategies(HugeConnectiveLabelStepStrategy.instance(),
+ HugeCountStrategy.instance(),
HugeVertexStepStrategy.instance(),
HugeGraphStepStrategy.instance(),
HugeCountStepStrategy.instance(),
diff --git
a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeConnectiveLabelStepStrategy.java
b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeConnectiveLabelStepStrategy.java
new file mode 100644
index 000000000..4a78ed252
--- /dev/null
+++
b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeConnectiveLabelStepStrategy.java
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hugegraph.traversal.optimize;
+
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Set;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.AndStep;
+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.util.HasContainer;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.InlineFilterStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
+
+public final class HugeConnectiveLabelStepStrategy
+ extends
AbstractTraversalStrategy<TraversalStrategy.OptimizationStrategy>
+ implements TraversalStrategy.OptimizationStrategy {
+
+ private static final long serialVersionUID = 2532355470697047377L;
+
+ private static final HugeConnectiveLabelStepStrategy INSTANCE =
+ new HugeConnectiveLabelStepStrategy();
+
+ private HugeConnectiveLabelStepStrategy() {
+ // pass
+ }
+
+ @Override
+ public void apply(Traversal.Admin<?, ?> traversal) {
+ Set<Traversal.Admin<?, ?>> visited =
+ Collections.newSetFromMap(new IdentityHashMap<>());
+ markConnectiveLabelSteps(traversal, visited);
+ }
+
+ @Override
+ public Set<Class<? extends TraversalStrategy.OptimizationStrategy>>
applyPost() {
+ return Collections.singleton(InlineFilterStrategy.class);
+ }
+
+ public static HugeConnectiveLabelStepStrategy instance() {
+ return INSTANCE;
+ }
+
+ private static void markConnectiveLabelSteps(Traversal.Admin<?, ?>
traversal,
+ Set<Traversal.Admin<?, ?>>
visited) {
+ if (!visited.add(traversal)) {
+ return;
+ }
+
+ for (AndStep<?> step : TraversalHelper.getStepsOfClass(AndStep.class,
+ traversal)) {
+ markConnectiveLabelChildren(step, step);
+ }
+ for (OrStep<?> step : TraversalHelper.getStepsOfClass(OrStep.class,
+ traversal)) {
+ markConnectiveLabelChildren(step, step);
+ }
+
+ for (Step<?, ?> step : traversal.getSteps()) {
+ if (!(step instanceof TraversalParent)) {
+ continue;
+ }
+ TraversalParent parent = (TraversalParent) step;
+ for (Traversal.Admin<?, ?> child : parent.getLocalChildren()) {
+ markConnectiveLabelSteps(child, visited);
+ }
+ for (Traversal.Admin<?, ?> child : parent.getGlobalChildren()) {
+ markConnectiveLabelSteps(child, visited);
+ }
+ }
+ }
+
+ private static void markConnectiveLabelChildren(Step<?, ?> step,
+ TraversalParent parent) {
+ if (!(step.getPreviousStep() instanceof HasStep)) {
+ return;
+ }
+ for (Traversal.Admin<?, ?> child : parent.getLocalChildren()) {
+ markPositiveLabelOnlyTraversal(child);
+ }
+ }
+
+ private static void markPositiveLabelOnlyTraversal(
+ Traversal.Admin<?, ?> traversal) {
+ for (Step<?, ?> step : traversal.getSteps()) {
+ if (!(step instanceof HasStep)) {
+ return;
+ }
+ HasStep<?> hasStep = (HasStep<?>) step;
+ if (!hasOnlyPositiveLabelContainers(hasStep)) {
+ return;
+ }
+ }
+
+ for (Step<?, ?> step : traversal.getSteps()) {
+ TraversalUtil.markConnectiveLabelStep(step);
+ }
+ }
+
+ private static boolean hasOnlyPositiveLabelContainers(HasStep<?> step) {
+ if (step.getHasContainers().isEmpty()) {
+ return false;
+ }
+ for (HasContainer has : step.getHasContainers()) {
+ if (!TraversalUtil.isPositiveLabelContainer(has)) {
+ return false;
+ }
+ }
+ return true;
+ }
+}
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 7b68f7177..e6a56027a 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
@@ -66,6 +66,7 @@ import
org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
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.map.CountGlobalStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep;
@@ -98,6 +99,9 @@ import com.google.common.collect.ImmutableList;
public final class TraversalUtil {
+ private static final String CONNECTIVE_LABEL_STEP =
+ "~hugegraph.connective-label-step";
+
public static final String P_CALL = "P.";
public static HugeGraph getGraph(Step<?, ?> step) {
@@ -174,26 +178,60 @@ public final class TraversalUtil {
Step<?, ?> nextStep = step.getNextStep();
if (step instanceof HasStep) {
HasContainerHolder holder = (HasContainerHolder) step;
+ boolean connectiveLabelStep =
+ removeConnectiveLabelStep(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.
+ * Range/neq predicates before match()/connective label filters
+ * may trigger a no-index query after nested filters add
+ * labels. Keep known-indexed predicates pushed down, and leave
+ * the rest for TinkerPop to evaluate.
*/
- if (followedByMatchStep(step) &&
- hasUnusableMatchPredicate(newStep, holder)) {
- List<HasContainer> extracted =
- extractUsableHasContainers(newStep, holder);
- for (HasContainer has : extracted) {
- holder.removeHasContainer(has);
+ boolean followedByMatch = followedByMatchStep(step);
+ Step<?, ?> afterPositiveLabelOrStep = null;
+ if (hasMatchIndexSensitivePredicate(holder)) {
+ afterPositiveLabelOrStep =
+ extractPositiveLabelOnlyOrStep(newStep,
+ traversal,
+ step);
+ if (afterPositiveLabelOrStep != null) {
+ nextStep = afterPositiveLabelOrStep;
}
- if (holder.getHasContainers().isEmpty()) {
- TraversalHelper.copyLabels(step,
step.getPreviousStep(),
- false);
- traversal.removeStep(step);
+ }
+ if (hasUnusableMatchPredicate(newStep, holder)) {
+ List<HasContainer> extracted;
+ if (followedByMatch) {
+ extracted = extractUsableHasContainers(newStep,
holder);
+ } else if (connectiveLabelStep &&
+ hasLabelAfterUnusablePredicate(newStep,
holder)) {
+ extracted = extractLabelHasContainers(newStep, holder);
+ } else {
+ if (afterPositiveLabelOrStep != null) {
+ step = nextStep;
+ continue;
+ }
+ if (hasUnsupportedLabelContainer(holder)) {
+ step = nextStep;
+ continue;
+ }
+ extracted = ImmutableList.of();
+ }
+ if (!extracted.isEmpty()) {
+ for (HasContainer has : extracted) {
+ holder.removeHasContainer(has);
+ }
+ if (holder.getHasContainers().isEmpty()) {
+ TraversalHelper.copyLabels(step,
+ step.getPreviousStep(),
+ false);
+ traversal.removeStep(step);
+ }
+ step = nextStep;
+ continue;
+ }
+ if (followedByMatch) {
+ step = nextStep;
+ continue;
}
- step = nextStep;
- continue;
}
if (extractHasContainers(newStep, holder)) {
TraversalHelper.copyLabels(step, step.getPreviousStep(),
false);
@@ -214,6 +252,167 @@ public final class TraversalUtil {
return next instanceof MatchStep;
}
+ private static Step<?, ?> extractPositiveLabelOnlyOrStep(
+ HugeGraphStep<?, ?> newStep, Traversal.Admin<?, ?> traversal,
+ Step<?, ?> step) {
+ OrStep<?> orStep = positiveLabelOnlyOrStepAfter(step);
+ if (orStep == null) {
+ return null;
+ }
+
+ List<Object> labels = new ArrayList<>();
+ for (Traversal.Admin<?, ?> child : orStep.getLocalChildren()) {
+ if (!collectPositiveLabelValues(child, labels)) {
+ return null;
+ }
+ }
+ if (labels.isEmpty()) {
+ return null;
+ }
+
+ HasContainer has = new HasContainer(T.label.getAccessor(),
+ P.within(labels));
+ if (!GraphStep.processHasContainerIds(newStep, has)) {
+ newStep.addHasContainer(has);
+ }
+
+ Step<?, ?> next = orStep.getNextStep();
+ TraversalHelper.copyLabels(orStep, orStep.getPreviousStep(), false);
+ traversal.removeStep(orStep);
+ return next;
+ }
+
+ private static OrStep<?> positiveLabelOnlyOrStepAfter(Step<?, ?> step) {
+ Step<?, ?> next = step.getNextStep();
+ while (next instanceof NoOpBarrierStep ||
+ next instanceof IdentityStep) {
+ next = next.getNextStep();
+ }
+ if (!(next instanceof OrStep)) {
+ return null;
+ }
+ return (OrStep<?>) next;
+ }
+
+ private static boolean collectPositiveLabelValues(
+ Traversal.Admin<?, ?> traversal, List<Object> labels) {
+ if (traversal.getSteps().size() != 1) {
+ return false;
+ }
+ Step<?, ?> step = traversal.getStartStep();
+ if (!(step instanceof HasStep)) {
+ return false;
+ }
+ HasStep<?> hasStep = (HasStep<?>) step;
+ if (hasStep.getHasContainers().size() != 1) {
+ return false;
+ }
+ HasContainer has = hasStep.getHasContainers().get(0);
+ if (!isPositiveLabelContainer(has)) {
+ return false;
+ }
+ addPositiveLabelValues(has, labels);
+ return true;
+ }
+
+ private static void addPositiveLabelValues(HasContainer has,
+ List<Object> labels) {
+ P<?> predicate = has.getPredicate();
+ BiPredicate<?, ?> bp = predicate.getBiPredicate();
+ if (bp == Compare.eq) {
+ labels.add(predicate.getValue());
+ } else {
+ assert bp == Contains.within;
+ labels.addAll((Collection<?>) predicate.getValue());
+ }
+ }
+
+ private static boolean hasLabelAfterUnusablePredicate(HugeGraphStep<?, ?>
step,
+ HasContainerHolder
holder) {
+ HugeGraph graph = tryGetGraph(step);
+ boolean seenUnusablePredicate = false;
+ for (HasContainer has : holder.getHasContainers()) {
+ if (isPositiveLabelContainer(has)) {
+ return seenUnusablePredicate;
+ }
+ if (hasMatchIndexSensitivePredicate(has) &&
+ (graph == null || !hasUsableMatchIndex(graph, step, has))) {
+ seenUnusablePredicate = true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean hasUnsupportedLabelContainer(
+ HasContainerHolder holder) {
+ for (HasContainer has : holder.getHasContainers()) {
+ if (isLabelContainer(has) && !isPositiveLabelContainer(has)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ static void markConnectiveLabelStep(Step<?, ?> step) {
+ step.addLabel(CONNECTIVE_LABEL_STEP);
+ }
+
+ private static boolean removeConnectiveLabelStep(Step<?, ?> step) {
+ boolean hasMarker = step.getLabels().contains(CONNECTIVE_LABEL_STEP);
+ if (hasMarker) {
+ step.removeLabel(CONNECTIVE_LABEL_STEP);
+ }
+ return hasMarker;
+ }
+
+ private static List<HasContainer> extractLabelHasContainers(
+ HugeGraphStep<?, ?> step, HasContainerHolder holder) {
+ List<HasContainer> extracted = new ArrayList<>();
+ for (HasContainer has : holder.getHasContainers()) {
+ if (!isPositiveLabelContainer(has)) {
+ continue;
+ }
+ if (!GraphStep.processHasContainerIds(step, has)) {
+ step.addHasContainer(has);
+ }
+ extracted.add(has);
+ }
+ return extracted;
+ }
+
+ private static boolean isLabelContainer(HasContainer has) {
+ return T.label.getAccessor().equals(has.getKey());
+ }
+
+ static boolean isPositiveLabelContainer(HasContainer has) {
+ if (!isLabelContainer(has)) {
+ return false;
+ }
+
+ P<?> predicate = has.getPredicate();
+ BiPredicate<?, ?> bp = predicate.getBiPredicate();
+ if (bp == Compare.eq) {
+ return true;
+ }
+ if (bp != Contains.within) {
+ return false;
+ }
+
+ Object value = predicate.getValue();
+ return value instanceof Collection &&
+ !((Collection<?>) value).isEmpty();
+ }
+
+ private static boolean hasMatchIndexSensitivePredicate(
+ HasContainerHolder holder) {
+ for (HasContainer has : holder.getHasContainers()) {
+ if (hasMatchIndexSensitivePredicate(has)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private static boolean hasUnusableMatchPredicate(HugeGraphStep<?, ?> step,
HasContainerHolder
holder) {
HugeGraph graph = tryGetGraph(step);
@@ -407,6 +606,7 @@ public final class TraversalUtil {
do {
Step<?, ?> nextStep = step.getNextStep();
if (step instanceof HasStep) {
+ removeConnectiveLabelStep(step);
HasContainerHolder holder = (HasContainerHolder) step;
if (extractHasContainers(newStep, holder)) {
TraversalHelper.copyLabels(step, step.getPreviousStep(),
false);
@@ -552,7 +752,7 @@ public final class TraversalUtil {
holder.setCount();
}
} while (step instanceof CountGlobalStep ||
- step instanceof FilterStep ||
+ (step instanceof FilterStep && !(step instanceof HasStep)) ||
step instanceof IdentityStep ||
step instanceof NoOpBarrierStep);
}
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 6e3167901..660c2e040 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
@@ -17,6 +17,7 @@
package org.apache.hugegraph.core;
+import org.apache.hugegraph.exception.NoIndexException;
import org.apache.hugegraph.schema.SchemaManager;
import org.apache.hugegraph.testutil.Assert;
import org.apache.hugegraph.traversal.optimize.HugeGraphStep;
@@ -120,6 +121,16 @@ public class CountStrategyCoreTest extends BaseCoreTest {
}
}
+ private void initConnectiveRangeNoIndexSchema() {
+ SchemaManager schema = graph().schema();
+ schema.propertyKey("ep4").asFloat().create();
+ schema.vertexLabel("vl1").create();
+ schema.edgeLabel("el2").properties("ep4")
+ .nullableKeys("ep4").link("vl1", "vl1").create();
+ schema.edgeLabel("el3").properties("ep4")
+ .nullableKeys("ep4").link("vl1", "vl1").create();
+ }
+
@Test
public void testWhereCountLtNegativeIsAlwaysFalse() {
this.initSchema();
@@ -286,6 +297,118 @@ public class CountStrategyCoreTest extends BaseCoreTest {
Assert.assertEquals(direct, viaMatch);
}
+ @Test
+ public void testConnectiveLabelAfterNoIndexRangeMatchesMatchTraversal() {
+ this.initConnectiveRangeNoIndexSchema();
+
+ Vertex v1 = graph().addVertex(T.label, "vl1");
+ Vertex v2 = graph().addVertex(T.label, "vl1");
+ Vertex v3 = graph().addVertex(T.label, "vl1");
+ v1.addEdge("el2", v2, "ep4", 0.1F);
+ v1.addEdge("el2", v3, "ep4", 0.5F);
+ v1.addEdge("el3", v2, "ep4", 0.1F);
+ commitTx();
+
+ Assert.assertEquals(2L, graph().traversal().E()
+ .hasLabel("el2").count().next());
+
+ GraphTraversal<Edge, Long> directTraversal = graph().traversal().E()
+ .has("ep4",
+
P.lt(0.32696354F))
+
.and(__.hasLabel("el2"))
+ .count();
+ HugeGraphStep<?, ?> graphStep = applyAndGetGraphStep(directTraversal);
+ Assert.assertEquals(1, graphStep.getHasContainers().size());
+ Assert.assertEquals(T.label.getAccessor(),
+ graphStep.getHasContainers().get(0).getKey());
+ Assert.assertTrue(hasRemainingHasStep(directTraversal, "ep4"));
+ long direct = directTraversal.next();
+ long viaMatch = graph().traversal().E()
+ .has("ep4", P.lt(0.32696354F))
+ .match(__.<Edge>as("start1")
+ .and(__.hasLabel("el2"))
+ .as("m1"))
+ .<Edge>select("m1").count().next();
+
+ Assert.assertEquals(1L, direct);
+ Assert.assertEquals(direct, viaMatch);
+ }
+
+ @Test
+ public void testOrConnectiveLabelsAfterNoIndexRangeMatchesLabelTraversal()
{
+ this.initConnectiveRangeNoIndexSchema();
+
+ Vertex v1 = graph().addVertex(T.label, "vl1");
+ Vertex v2 = graph().addVertex(T.label, "vl1");
+ Vertex v3 = graph().addVertex(T.label, "vl1");
+ v1.addEdge("el2", v2, "ep4", 0.1F);
+ v1.addEdge("el2", v3, "ep4", 0.5F);
+ v1.addEdge("el3", v2, "ep4", 0.1F);
+ commitTx();
+
+ long count = graph().traversal().E()
+ .has("ep4", P.lt(0.32696354F))
+ .or(__.hasLabel("el2"), __.hasLabel("el3"))
+ .count().next();
+
+ Assert.assertEquals(2L, count);
+ }
+
+ @Test
+ public void testPropertyBeforeLabelNoIndexRangeStillThrows() {
+ this.initConnectiveRangeNoIndexSchema();
+
+ Vertex v1 = graph().addVertex(T.label, "vl1");
+ Vertex v2 = graph().addVertex(T.label, "vl1");
+ v1.addEdge("el2", v2, "ep4", 0.1F);
+ commitTx();
+
+ Assert.assertThrows(NoIndexException.class, () -> {
+ graph().traversal().E()
+ .has("ep4", P.lt(0.32696354F))
+ .hasLabel("el2")
+ .count().next();
+ });
+ }
+
+ @Test
+ public void testNonLabelConnectiveAfterNoIndexRangeStillThrows() {
+ this.initConnectiveRangeNoIndexSchema();
+
+ Vertex v1 = graph().addVertex(T.label, "vl1");
+ Vertex v2 = graph().addVertex(T.label, "vl1");
+ v1.addEdge("el2", v2, "ep4", 0.1F);
+ commitTx();
+
+ Assert.assertThrows(NoIndexException.class, () -> {
+ graph().traversal().E()
+ .has("ep4", P.lt(0.32696354F))
+ .and(__.has("ep4", P.gt(0.0F)))
+ .count().next();
+ });
+ }
+
+ @Test
+ public void testNegativeConnectiveLabelAfterNoIndexRangeStaysLocal() {
+ this.initConnectiveRangeNoIndexSchema();
+
+ Vertex v1 = graph().addVertex(T.label, "vl1");
+ Vertex v2 = graph().addVertex(T.label, "vl1");
+ v1.addEdge("el2", v2, "ep4", 0.1F);
+ commitTx();
+
+ GraphTraversal<Edge, Edge> traversal = graph().traversal().E()
+ .has("ep4",
+ P.lt(0.32696354F))
+
.and(__.hasLabel(P.neq("el2")));
+
+ HugeGraphStep<?, ?> graphStep = applyAndGetGraphStep(traversal);
+ for (HasContainer has : graphStep.getHasContainers()) {
+ Assert.assertNotEquals(T.label.getAccessor(), has.getKey());
+ }
+ Assert.assertTrue(hasRemainingHasStep(traversal,
T.label.getAccessor()));
+ }
+
@Test
public void testMatchWithNoIndexConditionMatchesDirectTraversal() {
this.initMatchNoIndexSchema();
diff --git
a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtilOptimizeTest.java
b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtilOptimizeTest.java
index c3208a0b9..c79db5056 100644
---
a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtilOptimizeTest.java
+++
b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtilOptimizeTest.java
@@ -17,6 +17,9 @@
package org.apache.hugegraph.traversal.optimize;
+import java.util.Collections;
+import java.util.Set;
+
import org.apache.hugegraph.HugeGraph;
import org.apache.hugegraph.backend.id.Id;
import org.apache.hugegraph.backend.id.IdGenerator;
@@ -27,13 +30,19 @@ import org.apache.hugegraph.type.define.DataType;
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;
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.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.AndStep;
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.map.GraphStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.VertexStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.HasContainer;
+import
org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.InlineFilterStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
+import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.junit.Test;
import org.mockito.Mockito;
@@ -140,6 +149,86 @@ public class TraversalUtilOptimizeTest {
Assert.assertTrue(hasStepExists(traversal));
}
+ @Test
+ public void testExtractHasContainerExtractsPositiveLabelOnlyOrStep() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("age", P.gt(18))
+ .or(__.hasLabel("person"),
+
__.hasLabel(P.within("software")))
+ .asAdmin();
+ HugeGraphStep<?, ?> newStep = replaceGraphStep(traversal);
+
+ TraversalUtil.extractHasContainer(newStep, traversal);
+
+ Assert.assertTrue(hasContainer(newStep, T.label.getAccessor()));
+ Assert.assertTrue(hasStepExists(traversal, "age"));
+ Assert.assertFalse(stepExists(traversal, OrStep.class));
+ }
+
+ @Test
+ public void testExtractHasContainerKeepsUnsupportedOrLabelLocal() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("age", P.gt(18))
+ .or(__.hasLabel(P.neq("person")),
+ __.hasLabel("software"))
+ .asAdmin();
+ HugeGraphStep<?, ?> newStep = replaceGraphStep(traversal);
+
+ TraversalUtil.extractHasContainer(newStep, traversal);
+
+ Assert.assertFalse(hasContainer(newStep, T.label.getAccessor()));
+ Assert.assertTrue(hasStepExists(traversal, "age"));
+ Assert.assertTrue(stepExists(traversal, OrStep.class));
+ }
+
+ @Test
+ public void testExtractHasContainerKeepsNonLabelOrLocal() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("age", P.gt(18))
+ .or(__.has("name", "marko"),
+ __.hasLabel("software"))
+ .asAdmin();
+ HugeGraphStep<?, ?> newStep = replaceGraphStep(traversal);
+
+ TraversalUtil.extractHasContainer(newStep, traversal);
+
+ Assert.assertFalse(hasContainer(newStep, T.label.getAccessor()));
+ Assert.assertTrue(hasStepExists(traversal, "age"));
+ Assert.assertTrue(stepExists(traversal, OrStep.class));
+ }
+
+ @Test
+ public void testExtractHasContainerRequiresSingleLabelOnlyOrChild() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("age", P.gt(18))
+ .or(__.hasLabel("person")
+ .has("name", "marko"),
+ __.hasLabel("software"))
+ .asAdmin();
+ HugeGraphStep<?, ?> newStep = replaceGraphStep(traversal);
+
+ TraversalUtil.extractHasContainer(newStep, traversal);
+
+ Assert.assertFalse(hasContainer(newStep, T.label.getAccessor()));
+ Assert.assertTrue(hasStepExists(traversal, "age"));
+ Assert.assertTrue(stepExists(traversal, OrStep.class));
+ }
+
+ @Test
+ public void testExtractHasContainerSkipsOrWhenPredicateIsNotSensitive() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("name", "marko")
+ .or(__.hasLabel("person"),
+ __.hasLabel("software"))
+ .asAdmin();
+ HugeGraphStep<?, ?> newStep = replaceGraphStep(traversal);
+
+ TraversalUtil.extractHasContainer(newStep, traversal);
+
+ Assert.assertFalse(hasContainer(newStep, T.label.getAccessor()));
+ Assert.assertTrue(stepExists(traversal, OrStep.class));
+ }
+
@Test
public void testExtractHasContainerKeepsTextBetweenGraphHasStep() {
HugeGraph graph = Mockito.mock(HugeGraph.class);
@@ -207,6 +296,86 @@ public class TraversalUtilOptimizeTest {
Assert.assertFalse(hasStepExists(traversal));
}
+ @Test
+ public void testIsPositiveLabelContainer() {
+ Assert.assertTrue(TraversalUtil.isPositiveLabelContainer(
+ new HasContainer(T.label.getAccessor(), P.eq("person"))));
+ Assert.assertTrue(TraversalUtil.isPositiveLabelContainer(
+ new HasContainer(T.label.getAccessor(),
+ P.within("person", "software"))));
+
+ Assert.assertFalse(TraversalUtil.isPositiveLabelContainer(
+ new HasContainer("name", P.eq("person"))));
+ Assert.assertFalse(TraversalUtil.isPositiveLabelContainer(
+ new HasContainer(T.label.getAccessor(), P.neq("person"))));
+ Assert.assertFalse(TraversalUtil.isPositiveLabelContainer(
+ new HasContainer(T.label.getAccessor(),
+ P.without("person"))));
+ Assert.assertFalse(TraversalUtil.isPositiveLabelContainer(
+ new HasContainer(T.label.getAccessor(),
+ P.within(Collections.emptyList()))));
+ }
+
+ @Test
+ public void testConnectiveLabelStepStrategyApplyPost() {
+ Set<Class<? extends TraversalStrategy.OptimizationStrategy>> post =
+ HugeConnectiveLabelStepStrategy.instance().applyPost();
+
+ Assert.assertEquals(Collections.singleton(InlineFilterStrategy.class),
+ post);
+ }
+
+ @Test
+ public void testConnectiveLabelStepStrategyMarksAndChildren() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("age", P.gt(18))
+ .and(__.hasLabel("person"))
+ .asAdmin();
+
+ HugeConnectiveLabelStepStrategy.instance().apply(traversal);
+
+ Assert.assertTrue(hasMarkedLocalChild(traversal, AndStep.class));
+ }
+
+ @Test
+ public void testConnectiveLabelStepStrategyMarksOrChildren() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("age", P.gt(18))
+ .or(__.hasLabel("software"),
+ __.hasLabel(P.within("person")))
+ .asAdmin();
+
+ HugeConnectiveLabelStepStrategy.instance().apply(traversal);
+
+ Assert.assertTrue(hasMarkedLocalChild(traversal, OrStep.class));
+ }
+
+ @Test
+ public void testConnectiveLabelStepStrategySkipsWithoutPreviousHasStep() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .and(__.hasLabel("person"))
+ .asAdmin();
+
+ HugeConnectiveLabelStepStrategy.instance().apply(traversal);
+
+ Assert.assertFalse(hasMarkedLocalChild(traversal, AndStep.class));
+ }
+
+ @Test
+ public void testConnectiveLabelStepStrategySkipsUnsupportedChildren() {
+ Traversal.Admin<?, ?> traversal = __.V()
+ .has("age", P.gt(18))
+ .and(__.has("name", "marko"))
+
.or(__.hasLabel(P.without("person")),
+ __.has("name", "marko"))
+ .asAdmin();
+
+ HugeConnectiveLabelStepStrategy.instance().apply(traversal);
+
+ Assert.assertFalse(hasMarkedLocalChild(traversal, AndStep.class));
+ Assert.assertFalse(hasMarkedLocalChild(traversal, OrStep.class));
+ }
+
private static PropertyKey propertyKey(long id, String name,
DataType dataType) {
Id keyId = IdGenerator.of(id);
@@ -250,6 +419,15 @@ public class TraversalUtilOptimizeTest {
TraversalHelper.replaceStep((Step) origin, (Step) newStep, traversal);
}
+ private static boolean hasContainer(HugeGraphStep<?, ?> step, String key) {
+ for (HasContainer has : step.getHasContainers()) {
+ if (key.equals(has.getKey())) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private static boolean hasStepExists(Traversal.Admin<?, ?> traversal) {
for (Step<?, ?> step : traversal.getSteps()) {
if (step instanceof HasStep) {
@@ -258,4 +436,48 @@ public class TraversalUtilOptimizeTest {
}
return false;
}
+
+ private static boolean hasStepExists(Traversal.Admin<?, ?> traversal,
+ String key) {
+ for (Step<?, ?> step : traversal.getSteps()) {
+ if (!(step instanceof HasStep)) {
+ continue;
+ }
+ HasStep<?> hasStep = (HasStep<?>) step;
+ for (HasContainer has : hasStep.getHasContainers()) {
+ if (key.equals(has.getKey())) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private static boolean stepExists(Traversal.Admin<?, ?> traversal,
+ Class<?> clazz) {
+ for (Step<?, ?> step : traversal.getSteps()) {
+ if (clazz.isInstance(step)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean hasMarkedLocalChild(Traversal.Admin<?, ?> traversal,
+ Class<?> clazz) {
+ for (Step<?, ?> step : traversal.getSteps()) {
+ if (!clazz.isInstance(step)) {
+ continue;
+ }
+ TraversalParent parent = (TraversalParent) step;
+ for (Traversal.Admin<?, ?> child : parent.getLocalChildren()) {
+ for (Step<?, ?> childStep : child.getSteps()) {
+ if (!childStep.getLabels().isEmpty()) {
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ }
}
