Skip to content

Commit

Permalink
fix(interactive): Fix bugs in Label Name to Id Conversion (#3221)
Browse files Browse the repository at this point in the history 
<!--
Thanks for your contribution! please review
https://github.com/alibaba/GraphScope/blob/main/CONTRIBUTING.md before
opening an issue.
-->

## What do these changes do?
When converting a label name into an ID, it is necessary to first infer
whether it is a vertex label or an edge label based on the operator
preceding the elementMap. There will be issues when the preceding
operator falls into one of the following categories:
1. When the preceding operator is an `ExpandFusionStep` after
optimization (fusion of outE + filter + inV);
2. When the preceding operator is an operator like `limit` or `where`
that does not change the input type.

This PR addresses the two issues mentioned above.

<!-- Please give a short brief about these changes. -->

## Related issue number
fix #3219 

<!-- Are there any issues opened that will be resolved by merging this
change? -->

Fixes

Co-authored-by: Longbin Lai <[email protected]>
  • Loading branch information
@shirly121 @longbinlai
shirly121 and longbinlai authored Sep 14, 2023
1 parent 07104f4 commit 11d6ae6
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 16 deletions.
125 changes: 125 additions & 0 deletions ...in/java/com/alibaba/graphscope/gremlin/integration/suite/standard/IrGremlinQueryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ public abstract class IrGremlinQueryTest extends AbstractGremlinProcessTest {

public abstract Traversal<Vertex, Map<String, Object>> get_g_V_a_out_b_select_a_b_by_label_id();

public abstract Traversal<Vertex, Map<Object, Object>> get_g_V_outE_hasLabel_inV_elementMap();

public abstract Traversal<Vertex, Map<Object, Object>> get_g_V_limit_elementMap();

public abstract Traversal<Vertex, String> get_g_V_limit_label();

public abstract Traversal<Vertex, Map<String, Object>> get_g_V_a_out_b_select_by_elementMap();

public abstract Traversal<Vertex, Map<Object, Object>> get_g_V_group_by_by_sum();

public abstract Traversal<Vertex, Map<Object, Object>> get_g_V_group_by_by_max();
Expand Down Expand Up @@ -302,6 +310,97 @@ public void g_V_a_out_b_select_a_b_by_label_id() {
Assert.assertEquals("{a=person, b=lop}", traversal.next().toString());
}

@LoadGraphWith(LoadGraphWith.GraphData.MODERN)
@Test
public void g_V_outE_hasLabel_inV_elementMap() {
final Traversal<Vertex, Map<Object, Object>> traversal =
get_g_V_outE_hasLabel_inV_elementMap();
printTraversalForm(traversal);
int counter = 0;
while (traversal.hasNext()) {
Map values = traversal.next();
Assert.assertTrue(values.containsKey(T.id));
String name = (String) values.get("name");
if (name.equals("vadas")) {
Assert.assertEquals(27, values.get("age"));
Assert.assertEquals("person", values.get(T.label));
} else if (name.equals("josh")) {
Assert.assertEquals(32, values.get("age"));
Assert.assertEquals("person", values.get(T.label));
} else {
throw new IllegalStateException("It is not possible to reach here: " + values);
}
++counter;
}
Assert.assertEquals(2, counter);
}

@LoadGraphWith(LoadGraphWith.GraphData.MODERN)
@Test
public void g_V_limit_elementMap() {
final Traversal<Vertex, Map<Object, Object>> traversal = get_g_V_limit_elementMap();
printTraversalForm(traversal);
Map values = traversal.next();
Assert.assertTrue(values.containsKey(T.id));
String name = (String) values.get("name");
if (name.equals("marko")) {
Assert.assertEquals(29, values.get("age"));
Assert.assertEquals("person", values.get(T.label));
} else if (name.equals("josh")) {
Assert.assertEquals(32, values.get("age"));
Assert.assertEquals("person", values.get(T.label));
} else if (name.equals("peter")) {
Assert.assertEquals(35, values.get("age"));
Assert.assertEquals("person", values.get(T.label));
} else if (name.equals("vadas")) {
Assert.assertEquals(27, values.get("age"));
Assert.assertEquals("person", values.get(T.label));
} else if (name.equals("lop")) {
Assert.assertEquals("java", values.get("lang"));
Assert.assertEquals("software", values.get(T.label));
} else {
if (!name.equals("ripple")) {
throw new IllegalStateException("It is not possible to reach here: " + values);
}

Assert.assertEquals("java", values.get("lang"));
Assert.assertEquals("software", values.get(T.label));
}
}

@LoadGraphWith(LoadGraphWith.GraphData.MODERN)
@Test
public void g_V_limit_label() {
final Traversal<Vertex, String> traversal = get_g_V_limit_label();
printTraversalForm(traversal);
String value = traversal.next();
Assert.assertTrue(value.equals("person") || value.equals("software"));
}

@LoadGraphWith(LoadGraphWith.GraphData.MODERN)
@Test
public void g_V_a_out_b_select_by_elementMap() {
final Traversal<Vertex, Map<String, Object>> traversal =
get_g_V_a_out_b_select_by_elementMap();
printTraversalForm(traversal);

Map<String, Object> values = traversal.next();

Map value1 = (Map) values.get("a");
Assert.assertTrue(value1.containsKey(T.id));
Assert.assertEquals("marko", value1.get("name"));
Assert.assertEquals(29, value1.get("age"));
Assert.assertEquals("person", value1.get(T.label));

Map value2 = (Map) values.get("b");
Assert.assertTrue(value2.containsKey(T.id));
Assert.assertEquals("josh", value2.get("name"));
Assert.assertEquals(32, value2.get("age"));
Assert.assertEquals("person", value2.get(T.label));

Assert.assertTrue(!traversal.hasNext());
}

@LoadGraphWith(LoadGraphWith.GraphData.MODERN)
@Test
public void g_V_group_by_by_sum() {
Expand Down Expand Up @@ -640,6 +739,32 @@ public Traversal<Vertex, Map<String, Object>> get_g_V_a_out_b_select_a_b_by_labe
.by("name");
}

@Override
public Traversal<Vertex, Map<Object, Object>> get_g_V_outE_hasLabel_inV_elementMap() {
return g.V().outE().hasLabel("knows").inV().elementMap();
}

@Override
public Traversal<Vertex, Map<Object, Object>> get_g_V_limit_elementMap() {
return g.V().limit(1).elementMap();
}

@Override
public Traversal<Vertex, String> get_g_V_limit_label() {
return g.V().limit(1).label();
}

@Override
public Traversal<Vertex, Map<String, Object>> get_g_V_a_out_b_select_by_elementMap() {
return g.V().has("name", "marko")
.as("a")
.out()
.has("name", "josh")
.as("b")
.select("a", "b")
.by(elementMap());
}

@Override
public Traversal<Vertex, Map<Object, Object>> get_g_V_group_by_by_sum() {
return g.V().group().by().by(values("age").sum());
Expand Down
27 changes: 16 additions & 11 deletions ...e/compiler/src/main/java/com/alibaba/graphscope/gremlin/result/GremlinResultAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,22 +71,27 @@ public static GremlinResultParser analyze(Traversal traversal) {
parserType = UnionResultParser.create(step);
} else if (Utils.equalClass(step, SubgraphStep.class)) {
parserType = GremlinResultParserFactory.SUBGRAPH;
} else if (Utils.equalClass(step, HasStep.class)
|| Utils.equalClass(step, DedupGlobalStep.class)
|| Utils.equalClass(step, RangeGlobalStep.class)
|| Utils.equalClass(step, OrderGlobalStep.class)
|| Utils.equalClass(step, IsStep.class)
|| Utils.equalClass(step, WherePredicateStep.class)
|| Utils.equalClass(step, TraversalFilterStep.class)
|| Utils.equalClass(step, WhereTraversalStep.class)
|| Utils.equalClass(step, NotStep.class)
|| Utils.equalClass(step, CoinStep.class)
|| Utils.equalClass(step, SampleGlobalStep.class)) {
} else if (isSameInAndOutputType(step)) {
// do nothing;
} else {
throw new UnsupportedStepException(step.getClass(), "unimplemented yet");
}
}
return parserType;
}

// the step has the same input and output data type
public static boolean isSameInAndOutputType(Step step) {
return Utils.equalClass(step, HasStep.class)
|| Utils.equalClass(step, DedupGlobalStep.class)
|| Utils.equalClass(step, RangeGlobalStep.class)
|| Utils.equalClass(step, OrderGlobalStep.class)
|| Utils.equalClass(step, IsStep.class)
|| Utils.equalClass(step, WherePredicateStep.class)
|| Utils.equalClass(step, TraversalFilterStep.class)
|| Utils.equalClass(step, WhereTraversalStep.class)
|| Utils.equalClass(step, NotStep.class)
|| Utils.equalClass(step, CoinStep.class)
|| Utils.equalClass(step, SampleGlobalStep.class);
}
}
28 changes: 23 additions & 5 deletions ...tive_engine/compiler/src/main/java/com/alibaba/graphscope/gremlin/result/LabelParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.alibaba.graphscope.common.jna.type.FfiKeyType;
import com.alibaba.graphscope.gaia.proto.Common;
import com.alibaba.graphscope.gremlin.plugin.step.ExpandFusionStep;
import com.alibaba.graphscope.gremlin.transform.TraversalParentTransformFactory;
import com.google.common.collect.Lists;

Expand Down Expand Up @@ -130,15 +131,32 @@ private LabelType getLabelType(String tag, Object traversalOrStep, Step parent)
? parent
: (Step) traversalOrStep;
}
do {
tagStep = tagStep.getPreviousStep();
} while (tagStep != EmptyStep.instance()
&& !ObjectUtils.isEmpty(tag)
&& !tagStep.getLabels().contains(tag));
tagStep = tagStep.getPreviousStep();
while (tagStep != EmptyStep.instance()) {
if (ObjectUtils.isEmpty(tag) || tagStep.getLabels().contains(tag)) {
while (tagStep != EmptyStep.instance()
&& GremlinResultAnalyzer.isSameInAndOutputType(tagStep)) {
tagStep = tagStep.getPreviousStep();
}
break;
} else {
tagStep = tagStep.getPreviousStep();
}
}
if (tagStep instanceof GraphStep) {
return ((GraphStep) tagStep).returnsVertex()
? LabelType.VERTEX_LABEL
: LabelType.EDGE_LABEL;
} else if (tagStep instanceof ExpandFusionStep) {
switch (((ExpandFusionStep) tagStep).getExpandOpt()) {
case Vertex:
return LabelType.VERTEX_LABEL;
case Edge:
return LabelType.EDGE_LABEL;
case Degree:
default:
return LabelType.NONE;
}
} else if (tagStep instanceof VertexStep) {
return ((VertexStep) tagStep).returnsVertex()
? LabelType.VERTEX_LABEL
Expand Down

0 comments on commit 11d6ae6

Please sign in to comment.