Skip to content

Commit 79d62d4

Browse files
authored
fix(interactive): Fix unexpected alias when fusion Expand + GetV (#3676)
<!-- 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 #3671
1 parent ff30ce5 commit 79d62d4

File tree

10 files changed

+89
-45
lines changed

10 files changed

+89
-45
lines changed

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/rules/ExpandGetVFusionRule.java

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,13 @@
3737
// This rule try to fuse GraphLogicalExpand and GraphLogicalGetV if GraphLogicalExpand has no alias
3838
// (that it won't be visited individually later):
3939
// 1. if GraphLogicalGetV has no filters, then:
40-
// GraphLogicalExpand + GraphLogicalGetV -> GraphPhysicalExpand(ExpandV)
40+
// GraphLogicalExpand + GraphLogicalGetV -> GraphPhysicalExpand(ExpandV),
41+
// where GraphPhysicalExpand carries the alias of GraphLogicalGetV
4142
// 2. if GraphLogicalGetV has filters, then:
4243
// GraphLogicalExpand + GraphLogicalGetV -> GraphPhysicalExpand(ExpandV) +
43-
// GraphPhysicalGetV(VertexFilter)
44+
// GraphPhysicalGetV(VertexFilter),
45+
// where GraphPhysicalExpand carries the Default alias, and GraphPhysicalGetV carries the alias of
46+
// GraphLogicalGetV
4447
public abstract class ExpandGetVFusionRule<C extends RelRule.Config> extends RelRule<C>
4548
implements TransformationRule {
4649

@@ -54,18 +57,27 @@ protected RelNode transform(GraphLogicalGetV getV, GraphLogicalExpand expand, Re
5457
&& getV.getOpt().equals(GraphOpt.GetV.START)
5558
|| expand.getOpt().equals(GraphOpt.Expand.BOTH)
5659
&& getV.getOpt().equals(GraphOpt.GetV.OTHER)) {
57-
GraphPhysicalExpand physicalExpand =
58-
GraphPhysicalExpand.create(
59-
expand.getCluster(),
60-
expand.getHints(),
61-
input,
62-
expand,
63-
getV,
64-
GraphOpt.PhysicalExpandOpt.VERTEX,
65-
getV.getAliasName());
6660
if (ObjectUtils.isEmpty(getV.getFilters())) {
61+
GraphPhysicalExpand physicalExpand =
62+
GraphPhysicalExpand.create(
63+
expand.getCluster(),
64+
expand.getHints(),
65+
input,
66+
expand,
67+
getV,
68+
GraphOpt.PhysicalExpandOpt.VERTEX,
69+
getV.getAliasName());
6770
return physicalExpand;
6871
} else {
72+
GraphPhysicalExpand physicalExpand =
73+
GraphPhysicalExpand.create(
74+
expand.getCluster(),
75+
expand.getHints(),
76+
input,
77+
expand,
78+
getV,
79+
GraphOpt.PhysicalExpandOpt.VERTEX,
80+
AliasInference.DEFAULT_NAME);
6981
// If with filters, then create a GraphPhysicalGetV to do the filtering.
7082
// We set alias of getV to null to avoid alias conflict (with expand's alias)
7183
GraphPhysicalGetV physicalGetV =
@@ -74,7 +86,7 @@ protected RelNode transform(GraphLogicalGetV getV, GraphLogicalExpand expand, Re
7486
getV.getHints(),
7587
physicalExpand,
7688
getV,
77-
null,
89+
getV.getAliasName(),
7890
GraphOpt.PhysicalGetVOpt.ITSELF);
7991
return physicalGetV;
8092
}

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalExpand.java

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,31 @@ public static GraphPhysicalExpand create(
7171
GraphLogicalExpand fusedExpand,
7272
GraphLogicalGetV fusedGetV,
7373
GraphOpt.PhysicalExpandOpt physicalOpt,
74-
String alias) {
74+
String aliasName) {
75+
GraphLogicalGetV newGetV = null;
76+
if (fusedGetV != null) {
77+
// if fused to output vertices, build a new getV if a new aliasName is given, to make
78+
// sure the derived row type is correct (which is derived by getV)
79+
if (fusedGetV.getAliasName().equals(aliasName)) {
80+
newGetV = fusedGetV;
81+
} else {
82+
newGetV =
83+
GraphLogicalGetV.create(
84+
(GraphOptCluster) fusedGetV.getCluster(),
85+
fusedGetV.getHints(),
86+
input,
87+
fusedGetV.getOpt(),
88+
fusedGetV.getTableConfig(),
89+
aliasName,
90+
fusedGetV.getStartAlias());
91+
if (ObjectUtils.isNotEmpty(fusedGetV.getFilters())) {
92+
// should not have filters, as it would built as a new PhysicalGetV
93+
newGetV.setFilters(fusedGetV.getFilters());
94+
}
95+
}
96+
}
7597
return new GraphPhysicalExpand(
76-
cluster, hints, input, fusedExpand, fusedGetV, physicalOpt, alias);
98+
cluster, hints, input, fusedExpand, newGetV, physicalOpt, aliasName);
7799
}
78100

79101
public GraphOpt.PhysicalExpandOpt getPhysicalOpt() {
@@ -104,7 +126,7 @@ public int getAliasId() {
104126
public RelDataType deriveRowType() {
105127
switch (physicalOpt) {
106128
case EDGE:
107-
return fusedExpand.deriveRowType();
129+
return fusedExpand.getRowType();
108130
case DEGREE:
109131
{
110132
RelDataTypeFactory typeFactory = getCluster().getTypeFactory();
@@ -117,7 +139,7 @@ public RelDataType deriveRowType() {
117139
}
118140
case VERTEX:
119141
default:
120-
return fusedGetV.deriveRowType();
142+
return fusedGetV.getRowType();
121143
}
122144
}
123145

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalGetV.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.calcite.rel.RelWriter;
3131
import org.apache.calcite.rel.SingleRel;
3232
import org.apache.calcite.rel.hint.RelHint;
33+
import org.apache.calcite.rel.type.RelDataType;
3334
import org.apache.commons.lang3.ObjectUtils;
3435

3536
import java.util.List;
@@ -102,6 +103,11 @@ public List<RelNode> getInputs() {
102103
return this.input == null ? ImmutableList.of() : ImmutableList.of(this.input);
103104
}
104105

106+
@Override
107+
public RelDataType deriveRowType() {
108+
return fusedGetV.getRowType();
109+
}
110+
105111
@Override
106112
public RelWriter explainTerms(RelWriter pw) {
107113
return pw.itemIf("input", input, !Objects.isNull(input))

interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/plan/ExpandGetVFusionTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,10 @@ public void expand_getv_fusion_4_test() {
229229
planner.setRoot(before);
230230
RelNode after = planner.findBestExp();
231231
Assert.assertEquals(
232-
"GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[_],"
232+
"GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[a],"
233233
+ " fusedFilter=[[=(_.age, 10)]], opt=[END], physicalOpt=[ITSELF])\n"
234234
+ " GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}],"
235-
+ " alias=[a], opt=[OUT], physicalOpt=[VERTEX])\n"
235+
+ " alias=[_], opt=[OUT], physicalOpt=[VERTEX])\n"
236236
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
237237
+ " alias=[_], opt=[VERTEX])",
238238
after.explain().trim());
@@ -282,10 +282,10 @@ public void expand_getv_fusion_5_test() {
282282
planner.setRoot(before);
283283
RelNode after = planner.findBestExp();
284284
Assert.assertEquals(
285-
"GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[_],"
285+
"GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[a],"
286286
+ " fusedFilter=[[=(_.age, 10)]], opt=[END], physicalOpt=[ITSELF])\n"
287287
+ " GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}],"
288-
+ " alias=[a], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT],"
288+
+ " alias=[_], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT],"
289289
+ " physicalOpt=[VERTEX])\n"
290290
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
291291
+ " alias=[_], opt=[VERTEX])",

interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/runtime/GraphRelToProtoTest.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,7 @@ public void expand_degree_test() throws Exception {
863863
}
864864
}
865865

866-
// g.V().hasLabel("person").outE("knows").inV()
866+
// g.V().hasLabel("person").outE("knows").inV().as("a")
867867
@Test
868868
public void expand_vertex_test() throws Exception {
869869
GraphBuilder builder = Utils.mockGraphBuilder();
@@ -879,11 +879,12 @@ public void expand_vertex_test() throws Exception {
879879
.getV(
880880
new GetVConfig(
881881
GraphOpt.GetV.END,
882-
new LabelConfig(false).addLabel("person")))
882+
new LabelConfig(false).addLabel("person"),
883+
"a"))
883884
.build();
884885
Assert.assertEquals(
885886
"GraphLogicalGetV(tableConfig=[{isAll=false, tables=[person]}],"
886-
+ " alias=[_], opt=[END])\n"
887+
+ " alias=[a], opt=[END])\n"
887888
+ " GraphLogicalExpand(tableConfig=[{isAll=false, tables=[knows]}],"
888889
+ " alias=[_], opt=[OUT])\n"
889890
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
@@ -894,7 +895,7 @@ public void expand_vertex_test() throws Exception {
894895
planner.setRoot(before);
895896
RelNode after = planner.findBestExp();
896897
Assert.assertEquals(
897-
"GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}], alias=[_],"
898+
"GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}], alias=[a],"
898899
+ " opt=[OUT], physicalOpt=[VERTEX])\n"
899900
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
900901
+ " alias=[_], opt=[VERTEX])",
@@ -921,7 +922,7 @@ public void expand_vertex_test() throws Exception {
921922
}
922923
}
923924

924-
// g.V().hasLabel("person").outE("knows").inV().has("age",10), can be fused into
925+
// g.V().hasLabel("person").outE("knows").inV().as("a").has("age",10), can be fused into
925926
// GraphPhysicalExpand + GraphPhysicalGetV
926927
@Test
927928
public void expand_vertex_filter_test() throws Exception {
@@ -1015,10 +1016,10 @@ public void expand_vertex_with_filters_test() throws Exception {
10151016
planner.setRoot(before);
10161017
RelNode after = planner.findBestExp();
10171018
Assert.assertEquals(
1018-
"GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[_],"
1019+
"GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[a],"
10191020
+ " fusedFilter=[[=(_.age, 10)]], opt=[END], physicalOpt=[ITSELF])\n"
10201021
+ " GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}],"
1021-
+ " alias=[a], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT],"
1022+
+ " alias=[_], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT],"
10221023
+ " physicalOpt=[VERTEX])\n"
10231024
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
10241025
+ " alias=[_], opt=[VERTEX])",

interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_filter_test.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@
4545
"id": 0
4646
}],
4747
"sampleRatio": 1.0
48-
},
49-
"alias": 0
48+
}
5049
}
5150
},
5251
"metaData": [{
@@ -73,7 +72,8 @@
7372
}]
7473
}]
7574
}
76-
}
75+
},
76+
"alias": -1
7777
}]
7878
}, {
7979
"opr": {
@@ -113,7 +113,8 @@
113113
}]
114114
},
115115
"sampleRatio": 1.0
116-
}
116+
},
117+
"alias": 0
117118
}
118119
},
119120
"metaData": [{

interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_test.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@
4545
"id": 0
4646
}],
4747
"sampleRatio": 1.0
48-
}
48+
},
49+
"alias": 0
4950
}
5051
},
5152
"metaData": [{
@@ -72,8 +73,7 @@
7273
}]
7374
}]
7475
}
75-
},
76-
"alias": -1
76+
}
7777
}]
7878
}, {
7979
"opr": {

interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_with_filters_test.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@
7474
}]
7575
},
7676
"sampleRatio": 1.0
77-
},
78-
"alias": 0
77+
}
7978
}
8079
},
8180
"metaData": [{
@@ -102,7 +101,8 @@
102101
}]
103102
}]
104103
}
105-
}
104+
},
105+
"alias": -1
106106
}]
107107
}, {
108108
"opr": {
@@ -142,7 +142,8 @@
142142
}]
143143
},
144144
"sampleRatio": 1.0
145-
}
145+
},
146+
"alias": 0
146147
}
147148
},
148149
"metaData": [{

interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_filter_test.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@
5252
"id": 0
5353
}],
5454
"sampleRatio": 1.0
55-
},
56-
"alias": 0
55+
}
5756
}
5857
},
5958
"metaData": [{
@@ -80,7 +79,8 @@
8079
}]
8180
}]
8281
}
83-
}
82+
},
83+
"alias": -1
8484
}]
8585
}, {
8686
"opr": {
@@ -127,7 +127,8 @@
127127
}]
128128
},
129129
"sampleRatio": 1.0
130-
}
130+
},
131+
"alias": 0
131132
}
132133
},
133134
"metaData": [{

interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_test.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@
5252
"id": 0
5353
}],
5454
"sampleRatio": 1.0
55-
}
55+
},
56+
"alias": 0
5657
}
5758
},
5859
"metaData": [{
@@ -79,8 +80,7 @@
7980
}]
8081
}]
8182
}
82-
},
83-
"alias": -1
83+
}
8484
}]
8585
}, {
8686
"opr": {

0 commit comments

Comments
 (0)