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

chore: fix TinkerPop unit test #2055

Merged
merged 14 commits into from
Dec 26, 2022
Merged

chore: fix TinkerPop unit test #2055

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2830974
chore: don't support persistence for memory backend
zyxxoo Dec 18, 2022
b76394a
chore: setup graph by first step graph when graph is null
zyxxoo Dec 18, 2022
cd76807
chore: apply relations optimize logic
zyxxoo Dec 19, 2022
00e50be
chore: don't support null properties
zyxxoo Dec 19, 2022
c3317b1
chore: reflactor code
zyxxoo Dec 19, 2022
694a787
fix: g.V().has(empty).count() and g.V().count().as('a').select('a')
zyxxoo Dec 22, 2022
1e4a910
fix: specify the code generation directory for raft.proto (#2057)
simon824 Dec 21, 2022
02963d5
chore: reflactor code
zyxxoo Dec 22, 2022
62f9fed
fix: tinkerpop unit schema no define
zyxxoo Dec 22, 2022
ae7266d
chore: reflactor code
zyxxoo Dec 22, 2022
d2a494c
chore: improve code
zyxxoo Dec 22, 2022
9b5ee56
fix: core test
zyxxoo Dec 23, 2022
4a71089
Merge branch 'master' into zy_dev
zyxxoo Dec 26, 2022
225854c
fix the style & clean some outdated code
imbajin Dec 26, 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
1 change: 1 addition & 0 deletions hugegraph-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@
<protocArtifact>com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}</protocArtifact>
<pluginId>protoc-java</pluginId>
<protoSourceRoot>${project.basedir}/src/main/resources/proto</protoSourceRoot>
<outputDirectory>${basedir}/target/generated-sources/protobuf/java</outputDirectory>
</configuration>
<executions>
<execution>
Expand Down
3 changes: 2 additions & 1 deletion hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java
View file
Open in desktop
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
21 changes: 20 additions & 1 deletion hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/ConditionQueryFlatten.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static List<ConditionQuery> flatten(ConditionQuery query) {
public static List<ConditionQuery> flatten(ConditionQuery query,
boolean supportIn) {
if (query.isFlattened() && !query.mayHasDupKeys(SPECIAL_KEYS)) {
return ImmutableList.of(query);
return flattenRelations(query);
}

List<ConditionQuery> queries = new ArrayList<>();
Expand Down Expand Up @@ -251,6 +251,25 @@ private static ConditionQuery newQueryFromRelations(ConditionQuery query,
return cq;
}

private static List<ConditionQuery> flattenRelations(ConditionQuery query) {
Relations relations = new Relations();
List<Condition> nonRelations = new ArrayList<>();
for (Condition condition : query.conditions()) {
if (condition.isRelation()) {
relations.add((Relation) condition);
} else {
nonRelations.add(condition);
}
}
relations = optimizeRelations(relations);
if (relations != null) {
ConditionQuery cq = newQueryFromRelations(query, relations);
cq.query(nonRelations);
return ImmutableList.of(cq);
}
return ImmutableList.of(query);
}

private static Relations optimizeRelations(Relations relations) {
// Optimize and-relations in one query
// e.g. (age>1 and age>2) -> (age>2)
Expand Down
10 changes: 10 additions & 0 deletions hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeFeatures.java
View file
Open in desktop
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
1 change: 1 addition & 0 deletions ...aph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeCountStepStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public void apply(Traversal.Admin<?, ?> traversal) {
graphStep.queryInfo().aggregate(Aggregate.AggregateFunc.COUNT, null);
HugeCountStep<?> countStep = new HugeCountStep<>(traversal, graphStep);
for (Step<?, ?> origin : originSteps) {
TraversalHelper.copyLabels(origin, countStep, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a replaceStep method that contains a copyLabels call? otherwise add some comments for why?

traversal.removeStep(origin);
}
traversal.addStep(0, countStep);
Expand Down
4 changes: 4 additions & 0 deletions hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeGraphStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ public HugeGraphStep(final GraphStep<S, E> originGraphStep) {
}

protected long count() {
if (this.ids == null) {
return 0L;
}

if (this.returnsVertex()) {
return this.verticesCount();
} else {
Expand Down
3 changes: 3 additions & 0 deletions ...aph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugeGraphStepStrategy.java
View file
Open in desktop
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
97 changes: 90 additions & 7 deletions hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/TraversalUtil.java
View file
Open in desktop
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,73 @@ 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;
// TODO: remove these EmptyGraph judgments when upgrading tinkerpop to a fixed version
zyxxoo marked this conversation as resolved.
Show resolved Hide resolved
zyxxoo marked this conversation as resolved.
Show resolved Hide resolved
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;
}

// TODO: remove these EmptyGraph judgments when upgrading tinkerpop to a fixed version
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 +643,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
25 changes: 17 additions & 8 deletions hugegraph-test/src/main/java/org/apache/hugegraph/tinkerpop/TestGraph.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ public void initModernSchema(IdStrategy idStrategy) {
schema.propertyKey("ripple").ifNotExist().create();
schema.propertyKey("lop").ifNotExist().create();
schema.propertyKey("test").ifNotExist().create();
schema.propertyKey("p").ifNotExist().create();

switch (idStrategy) {
case AUTOMATIC:
Expand All @@ -475,8 +476,8 @@ public void initModernSchema(IdStrategy idStrategy) {
.nullableKeys("name")
.ifNotExist().create();
schema.vertexLabel(DEFAULT_VL)
.properties("name", "age")
.nullableKeys("name", "age")
.properties("name", "age", "p")
.nullableKeys("name", "age", "p")
.ifNotExist().create();
schema.vertexLabel("animal")
.properties("name", "age", "peter", "josh", "marko",
Expand All @@ -499,8 +500,8 @@ public void initModernSchema(IdStrategy idStrategy) {
.nullableKeys("name")
.useCustomizeStringId().ifNotExist().create();
schema.vertexLabel(DEFAULT_VL)
.properties("name", "age")
.nullableKeys("name", "age")
.properties("name", "age", "p")
.nullableKeys("name", "age", "p")
.useCustomizeStringId().ifNotExist().create();
schema.vertexLabel("animal")
.properties("name", "age")
Expand Down Expand Up @@ -557,6 +558,8 @@ public void initModernSchema(IdStrategy idStrategy) {
.range().ifNotExist().create();
schema.indexLabel("personByNameAge").onV("person").by("name", "age")
.ifNotExist().create();
schema.indexLabel("vertexByP").onV("vertex").by("p")
.ifNotExist().create();
}

@Watched
Expand Down Expand Up @@ -659,6 +662,8 @@ private void initBasicPropertyKey() {
schema.propertyKey("f").asFloat().ifNotExist().create();
schema.propertyKey("i").asInt().ifNotExist().create();
schema.propertyKey("l").asLong().ifNotExist().create();
schema.propertyKey("p").ifNotExist().create();
schema.propertyKey("k").ifNotExist().create();
schema.propertyKey("here").ifNotExist().create();
schema.propertyKey("to-change").ifNotExist().create();
schema.propertyKey("to-remove").ifNotExist().create();
Expand Down Expand Up @@ -704,7 +709,7 @@ private void initBasicVertexLabelV(IdStrategy idStrategy,
"favoriteColor", "aKey", "age", "boolean",
"float", "double", "string", "integer",
"long", "myId", "location", "x", "y", "s",
"n", "d", "f", "i", "l", "to-change",
"n", "d", "f", "i", "l", "p", "k", "to-change",
"to-remove", "to-keep", "old", "new",
"gremlin.partitionGraphStrategy.partition",
"color", "blah")
Expand All @@ -714,7 +719,7 @@ private void initBasicVertexLabelV(IdStrategy idStrategy,
"favoriteColor", "aKey", "age", "boolean",
"float", "double", "string", "integer",
"long", "myId", "location", "x", "y", "s",
"n", "d", "f", "i", "l", "to-change",
"n", "d", "f", "i", "l", "p", "k", "to-change",
"to-remove", "to-keep", "old", "new",
"gremlin.partitionGraphStrategy.partition",
"color", "blah")
Expand All @@ -729,7 +734,7 @@ private void initBasicVertexLabelV(IdStrategy idStrategy,
"favoriteColor", "aKey", "age", "boolean",
"float", "double", "string", "integer",
"long", "myId", "location", "x", "y", "s",
"n", "d", "f", "i", "l", "to-change",
"n", "d", "f", "i", "l", "p", "k", "to-change",
"to-remove", "to-keep", "old", "new",
"gremlin.partitionGraphStrategy.partition",
"color", "blah")
Expand All @@ -739,7 +744,7 @@ private void initBasicVertexLabelV(IdStrategy idStrategy,
"favoriteColor", "aKey", "age", "boolean",
"float", "double", "string", "integer",
"long", "myId", "location", "x", "y", "s",
"n", "d", "f", "i", "l", "to-change",
"n", "d", "f", "i", "l", "p", "k", "to-change",
"to-remove", "to-keep", "old", "new",
"gremlin.partitionGraphStrategy.partition",
"color", "blah")
Expand All @@ -754,6 +759,10 @@ private void initBasicVertexLabelV(IdStrategy idStrategy,
.ifNotExist().create();
schema.indexLabel("defaultVLByName").onV(defaultVL).by("name")
.ifNotExist().create();
schema.indexLabel("defaultVLBySome").onV(defaultVL).by("some")
.ifNotExist().create();
schema.indexLabel("defaultVLByK").onV(defaultVL).by("k")
.ifNotExist().create();
}

@Watched
Expand Down