Skip to content

Commit

Permalink
fix(interactive): Fix Type Inference Bugs in Path Expand After Optimi…
Browse files Browse the repository at this point in the history
…zation (#3889)

Type inference of path expand is bound to the user-given query order,
which may be reversed after optimization. Re-infer the type of path
expand after optimization if changes occur.

Fixes #3730
  • Loading branch information
shirly121 committed Jun 7, 2024
1 parent 5590f02 commit 9ff8c34
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -764,11 +764,10 @@ private RelNode createExpandGetV(
if (srcInTargetOrderId == null) {
srcInTargetOrderId = -1;
}
PatternVertex src =
glogueEdge.getSrcPattern().getVertexByOrder(edge.getSrcVertexOrder());
DataValue srcValue =
getVertexValue(
new VertexDataKey(srcInTargetOrderId),
edgeDetails,
glogueEdge.getSrcPattern().getVertexByOrder(edge.getSrcVertexOrder()));
getVertexValue(new VertexDataKey(srcInTargetOrderId), edgeDetails, src);
PatternVertex target =
glogueEdge.getDstPattern().getVertexByOrder(extendStep.getTargetVertexOrder());
DataValue targetValue =
Expand Down Expand Up @@ -801,7 +800,12 @@ private RelNode createExpandGetV(
GetVConfig innerGetVConfig =
new GetVConfig(
getVConfig.getOpt(),
createLabels(edge.getElementDetails().getPxdInnerGetVTypes(), true),
createLabels(
createInnerGetVTypes(
src.getVertexTypeIds(),
target.getVertexTypeIds(),
edge.getElementDetails()),
true),
getVConfig.getAlias());
pxdBuilder
.getV(innerGetVConfig)
Expand All @@ -828,9 +832,6 @@ private RelNode createExpandGetV(
getVConfig.getLabels(),
getVConfig.getAlias(),
getVConfig.getStartAlias()));
if (targetValue.getFilter() != null) {
builder.filter(targetValue.getFilter());
}
} else {
GraphLogicalExpand expand =
createExpandWithOptional(
Expand All @@ -844,13 +845,34 @@ private RelNode createExpandGetV(
builder.filter(edgeValue.getFilter());
}
builder.getV(getVConfig);
if (targetValue.getFilter() != null) {
builder.filter(targetValue.getFilter());
}
}
if (targetValue.getFilter() != null) {
builder.filter(targetValue.getFilter());
}
return builder.build();
}

// Handle a special case for path expand: re-infer the type of inner getV in path expand.
// Before optimization, the type of path expand is inferred based on the user's query order,
// resulting in the type of inner endV being bound to this order.
// However, if the optimizer reverses the order of the path expand, the type of inner endV
// also needs to be reversed accordingly.
private List<Integer> createInnerGetVTypes(
List<Integer> startVTypes, List<Integer> endVTypes, ElementDetails details) {
List<Integer> originalInnerVTypes = details.getPxdInnerGetVTypes();
if (!originalInnerVTypes.isEmpty()) {
if (!endVTypes.isEmpty() && originalInnerVTypes.containsAll(endVTypes)) {
return originalInnerVTypes;
} else if (!startVTypes.isEmpty() && originalInnerVTypes.containsAll(startVTypes)) {
List<Integer> reverseTypes = Lists.newArrayList(originalInnerVTypes);
reverseTypes.removeAll(startVTypes);
reverseTypes.addAll(endVTypes);
return reverseTypes.stream().distinct().collect(Collectors.toList());
}
}
return endVTypes;
}

private GraphLogicalPathExpand createPathExpandWithOptional(
GraphLogicalPathExpand pxd, boolean optional) {
if (pxd.getFused() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,4 +193,50 @@ public void Q4_test() {
+ " alias=[forum], opt=[VERTEX])",
com.alibaba.graphscope.common.ir.tools.Utils.toString(after).trim());
}

@Test
public void Q5_test() {
GraphBuilder builder = Utils.mockGraphBuilder(optimizer, irMeta);

// The optimized order is from 'b' to 'a', which is opposite to the user-given order.
// Verify that the path expand type is correctly inferred in this situation.
RelNode before1 =
com.alibaba.graphscope.cypher.antlr4.Utils.eval(
"Match (a)-[*1..2]->(b:COMMENT) Return a", builder)
.build();
RelNode after1 = optimizer.optimize(before1, new GraphIOProcessor(builder, irMeta));
Assert.assertEquals(
"GraphLogicalProject(a=[a], isAppend=[false])\n"
+ " GraphLogicalGetV(tableConfig=[{isAll=false, tables=[PERSON, COMMENT]}],"
+ " alias=[a], opt=[END])\n"
+ " GraphLogicalPathExpand(fused=[GraphPhysicalExpand(tableConfig=[[EdgeLabel(LIKES,"
+ " PERSON, COMMENT), EdgeLabel(REPLYOF, COMMENT, COMMENT)]], alias=[_],"
+ " opt=[IN], physicalOpt=[VERTEX])\n"
+ "], offset=[1], fetch=[1], path_opt=[ARBITRARY], result_opt=[END_V],"
+ " alias=[_], start_alias=[b])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[COMMENT]}],"
+ " alias=[b], opt=[VERTEX])",
after1.explain().trim());

// check the type of path expand if the order is from 'a' to 'b'
RelNode before2 =
com.alibaba.graphscope.cypher.antlr4.Utils.eval(
"Match (a {id: 1})-[*1..2]->(b:COMMENT) Return a", builder)
.build();
RelNode after2 = optimizer.optimize(before2, new GraphIOProcessor(builder, irMeta));
Assert.assertEquals(
"GraphLogicalProject(a=[a], isAppend=[false])\n"
+ " GraphLogicalGetV(tableConfig=[{isAll=false, tables=[COMMENT]}], alias=[b],"
+ " opt=[END])\n"
+ " GraphLogicalPathExpand(fused=[GraphPhysicalGetV(tableConfig=[{isAll=false,"
+ " tables=[COMMENT]}], alias=[_], opt=[END], physicalOpt=[ITSELF])\n"
+ " GraphPhysicalExpand(tableConfig=[[EdgeLabel(LIKES, PERSON, COMMENT),"
+ " EdgeLabel(REPLYOF, COMMENT, COMMENT)]], alias=[_], opt=[OUT],"
+ " physicalOpt=[VERTEX])\n"
+ "], offset=[1], fetch=[1], path_opt=[ARBITRARY], result_opt=[END_V],"
+ " alias=[_], start_alias=[a])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[PERSON,"
+ " COMMENT]}], alias=[a], fusedFilter=[[=(_.id, 1)]], opt=[VERTEX])",
after2.explain().trim());
}
}

0 comments on commit 9ff8c34

Please sign in to comment.