Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Traversal.graph is empty in StepStrategy.apply() with count().is(0) #1903

Merged
merged 2 commits into from
Jul 1, 2022
Merged

fix: Traversal.graph is empty in StepStrategy.apply() with count().is(0) #1903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4910d9d
fix: Traversal.graph is empty in StepStrategy.apply() with `count().i…
javeme Jun 13, 2022
64f0300
add test cases for count().is(0) and outE()/inE()/bothE().count()
javeme Jun 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions hugegraph-api/src/main/java/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@

import javax.security.sasl.AuthenticationException;

import com.baidu.hugegraph.iterator.MapperIterator;
import com.baidu.hugegraph.traversal.optimize.HugeScriptTraversal;
import jakarta.ws.rs.ForbiddenException;
import jakarta.ws.rs.NotAuthorizedException;

import org.apache.commons.configuration2.Configuration;
import org.apache.tinkerpop.gremlin.process.computer.GraphComputer;
import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
Expand Down Expand Up @@ -81,6 +76,7 @@
import com.baidu.hugegraph.config.TypedOption;
import com.baidu.hugegraph.exception.NotSupportException;
import com.baidu.hugegraph.iterator.FilterIterator;
import com.baidu.hugegraph.iterator.MapperIterator;
import com.baidu.hugegraph.rpc.RpcServiceConfig4Client;
import com.baidu.hugegraph.rpc.RpcServiceConfig4Server;
import com.baidu.hugegraph.schema.EdgeLabel;
Expand All @@ -98,6 +94,7 @@
import com.baidu.hugegraph.task.TaskManager;
import com.baidu.hugegraph.task.TaskScheduler;
import com.baidu.hugegraph.task.TaskStatus;
import com.baidu.hugegraph.traversal.optimize.HugeScriptTraversal;
import com.baidu.hugegraph.type.HugeType;
import com.baidu.hugegraph.type.Nameable;
import com.baidu.hugegraph.type.define.GraphMode;
Expand All @@ -107,6 +104,9 @@
import com.baidu.hugegraph.util.Log;
import com.baidu.hugegraph.util.RateLimiter;

import jakarta.ws.rs.ForbiddenException;
import jakarta.ws.rs.NotAuthorizedException;

public final class HugeGraphAuthProxy implements HugeGraph {

static {
Expand Down Expand Up @@ -1638,7 +1638,7 @@ public Iterator<TraversalStrategy<?>> iterator() {
return new MapperIterator<TraversalStrategy<?>,
TraversalStrategy<?>>(
this.strategies.iterator(), (strategy) -> {
return new TraversalStrategyProxy(strategy);
return new TraversalStrategyProxy<>(strategy);
});
}

Expand Down Expand Up @@ -1678,16 +1678,19 @@ private String translate(Bytecode bytecode) {
}
}

private final class TraversalStrategyProxy<T extends TraversalStrategy>
private final class TraversalStrategyProxy<T extends TraversalStrategy<?>>
implements TraversalStrategy<T> {

private static final long serialVersionUID = 2071829024642435735L;

private final TraversalStrategy<T> origin;

public TraversalStrategyProxy(TraversalStrategy<T> origin) {
this.origin = origin;
}

@Override
public void apply(Traversal.Admin traversal) {
public void apply(Traversal.Admin<?, ?> traversal) {
String script;
if (traversal instanceof HugeScriptTraversal) {
script = ((HugeScriptTraversal<?, ?>) traversal).script();
Expand Down Expand Up @@ -1740,9 +1743,9 @@ public Configuration getConfiguration() {
}

@Override
public int compareTo(Class<? extends TraversalStrategy>
otherTraversalCategory) {
return this.origin.compareTo(otherTraversalCategory);
public int compareTo(@SuppressWarnings("rawtypes")
Class<? extends TraversalStrategy> otherCategory) {
return this.origin.compareTo(otherCategory);
}

@Override
Expand Down
23 changes: 17 additions & 6 deletions ...aph-core/src/main/java/com/baidu/hugegraph/traversal/optimize/HugeVertexStepStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;

import com.baidu.hugegraph.HugeGraph;

public final class HugeVertexStepStrategy
extends AbstractTraversalStrategy<ProviderOptimizationStrategy>
implements ProviderOptimizationStrategy {
Expand Down Expand Up @@ -59,16 +61,25 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
if (!steps.isEmpty()) {
boolean withPath = HugeVertexStepStrategy.containsPath(traversal);
boolean withTree = HugeVertexStepStrategy.containsTree(traversal);
boolean supportIn = TraversalUtil.getGraph(steps.get(0))
.backendStoreFeatures()
.supportsQueryWithInCondition();
/*
* The graph of traversal may be null when `__` step is followed
* by `count().is(0)` step, like the following gremlin:
* `g.V(id).repeat(in()).until(or(inE().count().is(0), loops().is(2)))`
* TODO: remove this `graph!=null` check after fixed the bug #1699
*/
boolean supportIn = false;
HugeGraph graph = TraversalUtil.tryGetGraph(steps.get(0));
if (graph != null) {
supportIn = graph.backendStoreFeatures()
.supportsQueryWithInCondition();
}
batchOptimize = !withTree && !withPath && supportIn;
}

for (VertexStep originStep : steps) {
HugeVertexStep<?> newStep = batchOptimize ?
new HugeVertexStepByBatch<>(originStep) :
new HugeVertexStep<>(originStep);
new HugeVertexStepByBatch<>(originStep) :
new HugeVertexStep<>(originStep);
TraversalHelper.replaceStep(originStep, newStep, traversal);

TraversalUtil.extractHasContainer(newStep, traversal);
Expand Down Expand Up @@ -107,7 +118,7 @@ protected static boolean containsPath(Traversal.Admin<?, ?> traversal) {
*/
protected static boolean containsTree(Traversal.Admin<?, ?> traversal) {
boolean hasTree = TraversalHelper.getStepsOfClass(
TreeStep.class, traversal).size() > 0;
TreeStep.class, traversal).size() > 0;
if (hasTree) {
return true;
} else if (traversal instanceof EmptyTraversal) {
Expand Down
28 changes: 19 additions & 9 deletions hugegraph-core/src/main/java/com/baidu/hugegraph/traversal/optimize/TraversalUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,12 @@
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Element;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.Property;
import org.apache.tinkerpop.gremlin.structure.PropertyType;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.slf4j.Logger;
import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;

import com.baidu.hugegraph.HugeException;
import com.baidu.hugegraph.HugeGraph;
Expand All @@ -91,17 +92,27 @@
import com.baidu.hugegraph.util.DateUtil;
import com.baidu.hugegraph.util.E;
import com.baidu.hugegraph.util.JsonUtil;
import com.baidu.hugegraph.util.Log;
import com.google.common.collect.ImmutableList;

public final class TraversalUtil {

private static final Logger LOG = Log.logger(HugeGraph.class);

public static final String P_CALL = "P.";

public static HugeGraph getGraph(Step<?, ?> step) {
return (HugeGraph) step.getTraversal().getGraph().get();
HugeGraph graph = tryGetGraph(step);
if (graph != null) {
return graph;
}
throw new IllegalArgumentException("There is no graph in step: " + step);
}

public static HugeGraph tryGetGraph(Step<?, ?> step) {
Graph graph = step.getTraversal().getGraph().get();
if (graph instanceof HugeGraph) {
return (HugeGraph) graph;
}
assert graph == null || graph instanceof EmptyGraph;
return null;
}

public static void extractHasContainer(HugeGraphStep<?, ?> newStep,
Expand Down Expand Up @@ -572,10 +583,9 @@ public static void convAllHasSteps(Traversal.Admin<?, ?> traversal) {
TraversalHelper.getStepsOfAssignableClassRecursively(
HasStep.class, traversal);
/*
* The graph may be null.
* For example:
* g.V().hasLabel('person').union(__.<Vertex>has("birth", dates[0]))
* Here "__.has" will create a new traversal, but the graph is null
* The graph in traversal may be null, for example:
* `g.V().hasLabel('person').union(__.has('name', 'tom'))`
* Here `__.has()` will create a new traversal, but the graph is null
*/
if (steps.isEmpty() || !traversal.getGraph().isPresent()) {
return;
Expand Down
46 changes: 46 additions & 0 deletions hugegraph-test/src/main/java/com/baidu/hugegraph/core/EdgeCoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5354,6 +5354,52 @@ public void testQueryCount() {
.values().count().next());
}

@Test
public void testQueryCountOfAdjacentEdges() {
HugeGraph graph = graph();
GraphTraversalSource g = graph.traversal();
init18Edges();

Vertex james = vertex("author", "id", 1);

Assert.assertEquals(18L, g.E().count().next());

Assert.assertEquals(6L, g.V(james).bothE().count().next());

Assert.assertEquals(4L, g.V(james).outE().count().next());
Assert.assertEquals(1L, g.V(james).outE("created").count().next());
Assert.assertEquals(3L, g.V(james).outE("authored").count().next());
Assert.assertEquals(1L, g.V(james).outE("authored")
.has("score", 3).count().next());

Assert.assertEquals(2L, g.V(james).inE().count().next());
Assert.assertEquals(1L, g.V(james).inE("know").count().next());
Assert.assertEquals(1L, g.V(james).inE("follow").count().next());
}

@Test
public void testQueryCountAsCondition() {
HugeGraph graph = graph();
GraphTraversalSource g = graph.traversal();
init18Edges();

Vertex java1 = vertex("book", "name", "java-1");
Vertex java3 = vertex("book", "name", "java-3");

long paths;
paths = g.V(java1)
.repeat(__.inE().outV().simplePath())
.until(__.or(__.inE().count().is(0), __.loops().is(3)))
.path().count().next();
Assert.assertEquals(4L, paths);

paths = g.V(java3)
.repeat(__.inE().outV().simplePath())
.until(__.or(__.inE().count().is(0), __.loops().is(3)))
.path().count().next();
Assert.assertEquals(7L, paths);
}

@Test
public void testRemoveEdge() {
HugeGraph graph = graph();
Expand Down