Skip to content

Commit

Permalink
fix(interactive): fix the bug in missing metadata in PathExpand (#3667
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?

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

As titled.

## Related issue number

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

Fixes #3666
  • Loading branch information
@BingqingLyu
BingqingLyu authored Mar 29, 2024
1 parent b81e511 commit b2db3bd
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 72 deletions.
16 changes: 10 additions & 6 deletions ...rc/main/java/com/alibaba/graphscope/common/ir/runtime/proto/GraphRelToProtoConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@
import com.alibaba.graphscope.common.config.PegasusConfig;
import com.alibaba.graphscope.common.ir.meta.schema.CommonOptTable;
import com.alibaba.graphscope.common.ir.rel.*;
import com.alibaba.graphscope.common.ir.rel.GraphLogicalAggregate;
import com.alibaba.graphscope.common.ir.rel.GraphLogicalDedupBy;
import com.alibaba.graphscope.common.ir.rel.GraphLogicalProject;
import com.alibaba.graphscope.common.ir.rel.GraphLogicalSort;
import com.alibaba.graphscope.common.ir.rel.GraphShuttle;
import com.alibaba.graphscope.common.ir.rel.graph.*;
import com.alibaba.graphscope.common.ir.rel.graph.match.GraphLogicalMultiMatch;
import com.alibaba.graphscope.common.ir.rel.graph.match.GraphLogicalSingleMatch;
Expand Down Expand Up @@ -52,6 +47,7 @@
import org.apache.calcite.rel.logical.LogicalFilter;
import org.apache.calcite.rel.logical.LogicalJoin;
import org.apache.calcite.rel.logical.LogicalUnion;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rex.*;
import org.apache.calcite.sql.SqlKind;
Expand Down Expand Up @@ -220,6 +216,7 @@ public RelNode visit(GraphLogicalPathExpand pxd) {
GraphAlgebraPhysical.PathExpand.ExpandBase.Builder expandBaseBuilder =
GraphAlgebraPhysical.PathExpand.ExpandBase.newBuilder();
RelNode fused = pxd.getFused();
RelDataType rowType;
if (fused != null) {
// the case that expand base is fused
if (fused instanceof GraphPhysicalGetV) {
Expand All @@ -231,6 +228,7 @@ public RelNode visit(GraphLogicalPathExpand pxd) {
GraphPhysicalExpand fusedExpand = (GraphPhysicalExpand) fusedGetV.getInput();
GraphAlgebraPhysical.EdgeExpand.Builder expand = buildEdgeExpand(fusedExpand);
expandBaseBuilder.setEdgeExpand(expand);
rowType = fusedExpand.getRowType();
} else {
throw new UnsupportedOperationException(
"unsupported fused plan in path expand base: "
Expand All @@ -241,6 +239,7 @@ public RelNode visit(GraphLogicalPathExpand pxd) {
GraphPhysicalExpand fusedExpand = (GraphPhysicalExpand) fused;
GraphAlgebraPhysical.EdgeExpand.Builder expand = buildEdgeExpand(fusedExpand);
expandBaseBuilder.setEdgeExpand(expand);
rowType = fusedExpand.getFusedExpand().getRowType();
} else {
throw new UnsupportedOperationException(
"unsupported fused plan in path expand base");
Expand All @@ -252,6 +251,7 @@ public RelNode visit(GraphLogicalPathExpand pxd) {
GraphAlgebraPhysical.GetV.Builder getV = buildGetV((GraphLogicalGetV) pxd.getGetV());
expandBaseBuilder.setEdgeExpand(expand);
expandBaseBuilder.setGetV(getV);
rowType = pxd.getExpand().getRowType();
}
pathExpandBuilder.setBase(expandBaseBuilder);
pathExpandBuilder.setPathOpt(Utils.protoPathOpt(pxd.getPathOpt()));
Expand All @@ -266,6 +266,7 @@ public RelNode visit(GraphLogicalPathExpand pxd) {
}
oprBuilder.setOpr(
GraphAlgebraPhysical.PhysicalOpr.Operator.newBuilder().setPath(pathExpandBuilder));
oprBuilder.addAllMetaData(Utils.physicalProtoRowType(rowType, isColumnId));
if (isPartitioned) {
addRepartitionToAnother(pxd.getStartAlias().getAliasId());
}
Expand All @@ -281,8 +282,11 @@ public RelNode visit(GraphPhysicalExpand physicalExpand) {
GraphAlgebraPhysical.EdgeExpand.Builder edgeExpand = buildEdgeExpand(physicalExpand);
oprBuilder.setOpr(
GraphAlgebraPhysical.PhysicalOpr.Operator.newBuilder().setEdge(edgeExpand));
// Currently we use the row type of ExpandE as the output row type of the fused
// ExpandV, as desired by the engine implementation.
oprBuilder.addAllMetaData(
Utils.physicalProtoRowType(physicalExpand.getRowType(), isColumnId));
Utils.physicalProtoRowType(
physicalExpand.getFusedExpand().getRowType(), isColumnId));
if (isPartitioned) {
addRepartitionToAnother(physicalExpand.getStartAlias().getAliasId());
}
Expand Down
16 changes: 15 additions & 1 deletion interactive_engine/compiler/src/test/resources/proto/edge_expand_degree_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,21 @@
},
"metaData": [{
"type": {
"dataType": "INT64"
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "weight"
},
"type": "DOUBLE"
}]
}]
}
},
"alias": -1
}]
Expand Down
17 changes: 5 additions & 12 deletions interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_filter_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,17 @@
"metaData": [{
"type": {
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "id"
},
"type": "INT64"
}, {
"propId": {
"name": "name"
"name": "weight"
},
"type": "STRING"
}, {
"propId": {
"name": "age"
},
"type": "INT32"
"type": "DOUBLE"
}]
}]
}
Expand Down
20 changes: 7 additions & 13 deletions interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,28 +52,22 @@
"metaData": [{
"type": {
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "id"
},
"type": "INT64"
}, {
"propId": {
"name": "name"
"name": "weight"
},
"type": "STRING"
}, {
"propId": {
"name": "age"
},
"type": "INT32"
"type": "DOUBLE"
}]
}]
}
}
},
"alias": -1
}]
}, {
"opr": {
Expand Down
17 changes: 5 additions & 12 deletions ...active_engine/compiler/src/test/resources/proto/edge_expand_vertex_with_filters_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,24 +80,17 @@
"metaData": [{
"type": {
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "id"
},
"type": "INT64"
}, {
"propId": {
"name": "name"
"name": "weight"
},
"type": "STRING"
}, {
"propId": {
"name": "age"
},
"type": "INT32"
"type": "DOUBLE"
}]
}]
}
Expand Down
16 changes: 15 additions & 1 deletion ...ractive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_degree_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,21 @@
},
"metaData": [{
"type": {
"dataType": "INT64"
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "weight"
},
"type": "DOUBLE"
}]
}]
}
},
"alias": -1
}]
Expand Down
17 changes: 5 additions & 12 deletions ..._engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_filter_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,24 +58,17 @@
"metaData": [{
"type": {
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "id"
},
"type": "INT64"
}, {
"propId": {
"name": "name"
"name": "weight"
},
"type": "STRING"
}, {
"propId": {
"name": "age"
},
"type": "INT32"
"type": "DOUBLE"
}]
}]
}
Expand Down
20 changes: 7 additions & 13 deletions ...ractive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,28 +59,22 @@
"metaData": [{
"type": {
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "id"
},
"type": "INT64"
}, {
"propId": {
"name": "name"
"name": "weight"
},
"type": "STRING"
}, {
"propId": {
"name": "age"
},
"type": "INT32"
"type": "DOUBLE"
}]
}]
}
}
},
"alias": -1
}]
}, {
"opr": {
Expand Down
22 changes: 21 additions & 1 deletion interactive_engine/compiler/src/test/resources/proto/path_expand_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,27 @@
"pathOpt": "SIMPLE",
"resultOpt": "ALL_V"
}
}
},
"metaData": [{
"type": {
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "weight"
},
"type": "DOUBLE"
}]
}]
}
},
"alias": -1
}]
}, {
"opr": {
"sink": {
Expand Down
22 changes: 21 additions & 1 deletion interactive_engine/compiler/src/test/resources/proto/path_fused_expand_test.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,27 @@
"pathOpt": "SIMPLE",
"resultOpt": "ALL_V"
}
}
},
"metaData": [{
"type": {
"graphType": {
"elementOpt": "EDGE",
"graphDataType": [{
"label": {
"srcLabel": 0,
"dstLabel": 0
},
"props": [{
"propId": {
"name": "weight"
},
"type": "DOUBLE"
}]
}]
}
},
"alias": -1
}]
}, {
"opr": {
"sink": {
Expand Down

0 comments on commit b2db3bd

Please sign in to comment.