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.

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

chore: fix TinkerPop unit test #2055

Merged
merged 14 commits into from
Dec 26, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ public StandardHugeGraph(HugeConfig config) {

this.taskManager = TaskManager.instance();

this.features = new HugeFeatures(this, true);
boolean supportsPersistence = !"memory".equals(config.get(CoreOptions.BACKEND));
zyxxoo marked this conversation as resolved.
Show resolved Hide resolved
zyxxoo marked this conversation as resolved.
Show resolved Hide resolved
this.features = new HugeFeatures(this, supportsPersistence);

this.name = config.get(CoreOptions.STORE);
this.started = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,22 @@ public static List<ConditionQuery> flatten(ConditionQuery query) {
public static List<ConditionQuery> flatten(ConditionQuery query,
boolean supportIn) {
if (query.isFlattened() && !query.mayHasDupKeys(SPECIAL_KEYS)) {
Relations relations = new Relations();
List<Condition> noRelations = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to conditions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are two type, the one is relation, and other, so maybe relation name is more suitable.

for (Condition condition : query.conditions()) {
if (condition.isRelation()) {
relations.add((Relation) condition);
} else {
noRelations.add(condition);
}
}
relations = optimizeRelations(relations);
if (relations != null) {
ConditionQuery cq = newQueryFromRelations(query, relations);
cq.query(noRelations);
return ImmutableList.of(cq);
}
zyxxoo marked this conversation as resolved.
Show resolved Hide resolved

return ImmutableList.of(query);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ public VertexProperty.Cardinality getCardinality(final String key) {
return VertexProperty.Cardinality.single;
}

@Override
public boolean supportsNullPropertyValues() {
return false;
}

public boolean supportsDefaultLabel() {
return true;
}
Expand All @@ -291,6 +296,11 @@ public class HugeEdgeFeatures extends HugeElementFeatures
public EdgePropertyFeatures properties() {
return this.edgePropertyFeatures;
}

@Override
public boolean supportsNullPropertyValues() {
return false;
}
}

public class HugeDataTypeFeatures implements DataTypeFeatures {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ public void apply(Traversal.Admin<?, ?> traversal) {
List<GraphStep> steps = TraversalHelper.getStepsOfClass(
GraphStep.class, traversal);
for (GraphStep originStep : steps) {
TraversalUtil.trySetGraph(originStep,
TraversalUtil.tryGetGraph(steps.get(0)));

HugeGraphStep<?, ?> newStep = new HugeGraphStep<>(originStep);
TraversalHelper.replaceStep(originStep, newStep, traversal);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -54,6 +55,7 @@
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
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.RangeGlobalStep;
Expand Down Expand Up @@ -105,12 +107,71 @@ public static HugeGraph getGraph(Step<?, ?> step) {
}

public static HugeGraph tryGetGraph(Step<?, ?> step) {
Graph graph = step.getTraversal().getGraph().get();
if (graph instanceof HugeGraph) {
return (HugeGraph) graph;
Optional<Graph> graph = step.getTraversal()
.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
zyxxoo marked this conversation as resolved.
Show resolved Hide resolved
});
if (!graph.isPresent()) {
TraversalParent parent = step.getTraversal().getParent();
if (parent instanceof Traversal) {
Optional<Graph> parentGraph = ((Traversal<?, ?>) parent)
.asAdmin()
.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
});
if (parentGraph.isPresent()) {
step.getTraversal().setGraph(parentGraph.get());
return (HugeGraph) parentGraph.get();
}
}

return null;
}
assert graph == null || graph instanceof EmptyGraph;
return null;

assert graph.get() instanceof HugeGraph;
return (HugeGraph) graph.get();
}

public static void trySetGraph(Step<?, ?> step, HugeGraph graph) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems don't need to set graph anymore, please refer to apache/tinkerpop#1699

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I have add TODO
// TODO: remove these EmptyGraph judgments when upgrading tinkerpop to a fixed version

if (graph == null || step == null || step.getTraversal() == null) {
return;
}

Optional<Graph> stepGraph = step.getTraversal()
.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
});

if (step instanceof TraversalParent) {
for (final Traversal.Admin<?, ?> local : ((TraversalParent) step).getLocalChildren()) {
if (local.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
continue;
}
local.setGraph(graph);
}
for (final Traversal.Admin<?, ?> global : ((TraversalParent) step).getGlobalChildren()) {
if (global.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
continue;
}
global.setGraph(graph);
}
}

if (stepGraph.isPresent()) {
assert stepGraph.get() instanceof HugeGraph;
return;
}

step.getTraversal().setGraph(graph);
}

public static void extractHasContainer(HugeGraphStep<?, ?> newStep,
Expand Down Expand Up @@ -580,14 +641,34 @@ public static void convAllHasSteps(Traversal.Admin<?, ?> traversal) {
List<HasStep> steps =
TraversalHelper.getStepsOfAssignableClassRecursively(
HasStep.class, traversal);

if (steps.isEmpty()) {
return;
}

/*
* 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;
if (!traversal.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
if (traversal.getParent() == null || !(traversal.getParent() instanceof Traversal)) {
return;
}

Optional<Graph> parentGraph = ((Traversal<?, ?>) traversal.getParent())
.asAdmin()
.getGraph();
if (parentGraph.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
traversal.setGraph(parentGraph.get());
}
}

HugeGraph graph = (HugeGraph) traversal.getGraph().get();
for (HasStep<?> step : steps) {
TraversalUtil.convHasStep(graph, step);
Expand Down