From 79d62d41bf182637ee219388a9ce490733866023 Mon Sep 17 00:00:00 2001 From: BingqingLyu Date: Fri, 29 Mar 2024 11:34:08 +0800 Subject: [PATCH] fix(interactive): Fix unexpected alias when fusion `Expand` + `GetV` (#3676) ## What do these changes do? As titled. ## Related issue number Fixes #3671 --- .../planner/rules/ExpandGetVFusionRule.java | 36 ++++++++++++------- .../ir/rel/graph/GraphPhysicalExpand.java | 30 +++++++++++++--- .../ir/rel/graph/GraphPhysicalGetV.java | 6 ++++ .../common/ir/plan/ExpandGetVFusionTest.java | 8 ++--- .../ir/runtime/GraphRelToProtoTest.java | 15 ++++---- .../proto/edge_expand_vertex_filter_test.json | 9 ++--- .../proto/edge_expand_vertex_test.json | 6 ++-- .../edge_expand_vertex_with_filters_test.json | 9 ++--- ...tioned_edge_expand_vertex_filter_test.json | 9 ++--- .../partitioned_edge_expand_vertex_test.json | 6 ++-- 10 files changed, 89 insertions(+), 45 deletions(-) diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/rules/ExpandGetVFusionRule.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/rules/ExpandGetVFusionRule.java index fa2d9c288651..71c796a45f4d 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/rules/ExpandGetVFusionRule.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/rules/ExpandGetVFusionRule.java @@ -37,10 +37,13 @@ // This rule try to fuse GraphLogicalExpand and GraphLogicalGetV if GraphLogicalExpand has no alias // (that it won't be visited individually later): // 1. if GraphLogicalGetV has no filters, then: -// GraphLogicalExpand + GraphLogicalGetV -> GraphPhysicalExpand(ExpandV) +// GraphLogicalExpand + GraphLogicalGetV -> GraphPhysicalExpand(ExpandV), +// where GraphPhysicalExpand carries the alias of GraphLogicalGetV // 2. if GraphLogicalGetV has filters, then: // GraphLogicalExpand + GraphLogicalGetV -> GraphPhysicalExpand(ExpandV) + -// GraphPhysicalGetV(VertexFilter) +// GraphPhysicalGetV(VertexFilter), +// where GraphPhysicalExpand carries the Default alias, and GraphPhysicalGetV carries the alias of +// GraphLogicalGetV public abstract class ExpandGetVFusionRule extends RelRule implements TransformationRule { @@ -54,18 +57,27 @@ protected RelNode transform(GraphLogicalGetV getV, GraphLogicalExpand expand, Re && getV.getOpt().equals(GraphOpt.GetV.START) || expand.getOpt().equals(GraphOpt.Expand.BOTH) && getV.getOpt().equals(GraphOpt.GetV.OTHER)) { - GraphPhysicalExpand physicalExpand = - GraphPhysicalExpand.create( - expand.getCluster(), - expand.getHints(), - input, - expand, - getV, - GraphOpt.PhysicalExpandOpt.VERTEX, - getV.getAliasName()); if (ObjectUtils.isEmpty(getV.getFilters())) { + GraphPhysicalExpand physicalExpand = + GraphPhysicalExpand.create( + expand.getCluster(), + expand.getHints(), + input, + expand, + getV, + GraphOpt.PhysicalExpandOpt.VERTEX, + getV.getAliasName()); return physicalExpand; } else { + GraphPhysicalExpand physicalExpand = + GraphPhysicalExpand.create( + expand.getCluster(), + expand.getHints(), + input, + expand, + getV, + GraphOpt.PhysicalExpandOpt.VERTEX, + AliasInference.DEFAULT_NAME); // If with filters, then create a GraphPhysicalGetV to do the filtering. // We set alias of getV to null to avoid alias conflict (with expand's alias) GraphPhysicalGetV physicalGetV = @@ -74,7 +86,7 @@ protected RelNode transform(GraphLogicalGetV getV, GraphLogicalExpand expand, Re getV.getHints(), physicalExpand, getV, - null, + getV.getAliasName(), GraphOpt.PhysicalGetVOpt.ITSELF); return physicalGetV; } diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalExpand.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalExpand.java index 478586969cd7..84a47262362c 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalExpand.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalExpand.java @@ -71,9 +71,31 @@ public static GraphPhysicalExpand create( GraphLogicalExpand fusedExpand, GraphLogicalGetV fusedGetV, GraphOpt.PhysicalExpandOpt physicalOpt, - String alias) { + String aliasName) { + GraphLogicalGetV newGetV = null; + if (fusedGetV != null) { + // if fused to output vertices, build a new getV if a new aliasName is given, to make + // sure the derived row type is correct (which is derived by getV) + if (fusedGetV.getAliasName().equals(aliasName)) { + newGetV = fusedGetV; + } else { + newGetV = + GraphLogicalGetV.create( + (GraphOptCluster) fusedGetV.getCluster(), + fusedGetV.getHints(), + input, + fusedGetV.getOpt(), + fusedGetV.getTableConfig(), + aliasName, + fusedGetV.getStartAlias()); + if (ObjectUtils.isNotEmpty(fusedGetV.getFilters())) { + // should not have filters, as it would built as a new PhysicalGetV + newGetV.setFilters(fusedGetV.getFilters()); + } + } + } return new GraphPhysicalExpand( - cluster, hints, input, fusedExpand, fusedGetV, physicalOpt, alias); + cluster, hints, input, fusedExpand, newGetV, physicalOpt, aliasName); } public GraphOpt.PhysicalExpandOpt getPhysicalOpt() { @@ -104,7 +126,7 @@ public int getAliasId() { public RelDataType deriveRowType() { switch (physicalOpt) { case EDGE: - return fusedExpand.deriveRowType(); + return fusedExpand.getRowType(); case DEGREE: { RelDataTypeFactory typeFactory = getCluster().getTypeFactory(); @@ -117,7 +139,7 @@ public RelDataType deriveRowType() { } case VERTEX: default: - return fusedGetV.deriveRowType(); + return fusedGetV.getRowType(); } } diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalGetV.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalGetV.java index 9d044ed7db77..540e67acb00d 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalGetV.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalGetV.java @@ -30,6 +30,7 @@ import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.SingleRel; import org.apache.calcite.rel.hint.RelHint; +import org.apache.calcite.rel.type.RelDataType; import org.apache.commons.lang3.ObjectUtils; import java.util.List; @@ -102,6 +103,11 @@ public List getInputs() { return this.input == null ? ImmutableList.of() : ImmutableList.of(this.input); } + @Override + public RelDataType deriveRowType() { + return fusedGetV.getRowType(); + } + @Override public RelWriter explainTerms(RelWriter pw) { return pw.itemIf("input", input, !Objects.isNull(input)) diff --git a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/plan/ExpandGetVFusionTest.java b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/plan/ExpandGetVFusionTest.java index 9b4deda35a59..5d2835fc0653 100644 --- a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/plan/ExpandGetVFusionTest.java +++ b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/plan/ExpandGetVFusionTest.java @@ -229,10 +229,10 @@ public void expand_getv_fusion_4_test() { planner.setRoot(before); RelNode after = planner.findBestExp(); Assert.assertEquals( - "GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[_]," + "GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[a]," + " fusedFilter=[[=(_.age, 10)]], opt=[END], physicalOpt=[ITSELF])\n" + " GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}]," - + " alias=[a], opt=[OUT], physicalOpt=[VERTEX])\n" + + " alias=[_], opt=[OUT], physicalOpt=[VERTEX])\n" + " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}]," + " alias=[_], opt=[VERTEX])", after.explain().trim()); @@ -282,10 +282,10 @@ public void expand_getv_fusion_5_test() { planner.setRoot(before); RelNode after = planner.findBestExp(); Assert.assertEquals( - "GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[_]," + "GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[a]," + " fusedFilter=[[=(_.age, 10)]], opt=[END], physicalOpt=[ITSELF])\n" + " GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}]," - + " alias=[a], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT]," + + " alias=[_], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT]," + " physicalOpt=[VERTEX])\n" + " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}]," + " alias=[_], opt=[VERTEX])", diff --git a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/runtime/GraphRelToProtoTest.java b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/runtime/GraphRelToProtoTest.java index ef48388f0357..ab620ae84803 100644 --- a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/runtime/GraphRelToProtoTest.java +++ b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/common/ir/runtime/GraphRelToProtoTest.java @@ -863,7 +863,7 @@ public void expand_degree_test() throws Exception { } } - // g.V().hasLabel("person").outE("knows").inV() + // g.V().hasLabel("person").outE("knows").inV().as("a") @Test public void expand_vertex_test() throws Exception { GraphBuilder builder = Utils.mockGraphBuilder(); @@ -879,11 +879,12 @@ public void expand_vertex_test() throws Exception { .getV( new GetVConfig( GraphOpt.GetV.END, - new LabelConfig(false).addLabel("person"))) + new LabelConfig(false).addLabel("person"), + "a")) .build(); Assert.assertEquals( "GraphLogicalGetV(tableConfig=[{isAll=false, tables=[person]}]," - + " alias=[_], opt=[END])\n" + + " alias=[a], opt=[END])\n" + " GraphLogicalExpand(tableConfig=[{isAll=false, tables=[knows]}]," + " alias=[_], opt=[OUT])\n" + " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}]," @@ -894,7 +895,7 @@ public void expand_vertex_test() throws Exception { planner.setRoot(before); RelNode after = planner.findBestExp(); Assert.assertEquals( - "GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}], alias=[_]," + "GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}], alias=[a]," + " opt=[OUT], physicalOpt=[VERTEX])\n" + " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}]," + " alias=[_], opt=[VERTEX])", @@ -921,7 +922,7 @@ public void expand_vertex_test() throws Exception { } } - // g.V().hasLabel("person").outE("knows").inV().has("age",10), can be fused into + // g.V().hasLabel("person").outE("knows").inV().as("a").has("age",10), can be fused into // GraphPhysicalExpand + GraphPhysicalGetV @Test public void expand_vertex_filter_test() throws Exception { @@ -1015,10 +1016,10 @@ public void expand_vertex_with_filters_test() throws Exception { planner.setRoot(before); RelNode after = planner.findBestExp(); Assert.assertEquals( - "GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[_]," + "GraphPhysicalGetV(tableConfig=[{isAll=false, tables=[person]}], alias=[a]," + " fusedFilter=[[=(_.age, 10)]], opt=[END], physicalOpt=[ITSELF])\n" + " GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}]," - + " alias=[a], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT]," + + " alias=[_], fusedFilter=[[=(_.weight, 5E-1)]], opt=[OUT]," + " physicalOpt=[VERTEX])\n" + " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}]," + " alias=[_], opt=[VERTEX])", diff --git a/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_filter_test.json b/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_filter_test.json index c5331ee3a18e..b46ab5eedfea 100644 --- a/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_filter_test.json +++ b/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_filter_test.json @@ -45,8 +45,7 @@ "id": 0 }], "sampleRatio": 1.0 - }, - "alias": 0 + } } }, "metaData": [{ @@ -73,7 +72,8 @@ }] }] } - } + }, + "alias": -1 }] }, { "opr": { @@ -113,7 +113,8 @@ }] }, "sampleRatio": 1.0 - } + }, + "alias": 0 } }, "metaData": [{ diff --git a/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_test.json b/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_test.json index 9354d27c58b4..f848b668e9e2 100644 --- a/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_test.json +++ b/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_test.json @@ -45,7 +45,8 @@ "id": 0 }], "sampleRatio": 1.0 - } + }, + "alias": 0 } }, "metaData": [{ @@ -72,8 +73,7 @@ }] }] } - }, - "alias": -1 + } }] }, { "opr": { diff --git a/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_with_filters_test.json b/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_with_filters_test.json index 7b7dcc13999e..441a7f5b7428 100644 --- a/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_with_filters_test.json +++ b/interactive_engine/compiler/src/test/resources/proto/edge_expand_vertex_with_filters_test.json @@ -74,8 +74,7 @@ }] }, "sampleRatio": 1.0 - }, - "alias": 0 + } } }, "metaData": [{ @@ -102,7 +101,8 @@ }] }] } - } + }, + "alias": -1 }] }, { "opr": { @@ -142,7 +142,8 @@ }] }, "sampleRatio": 1.0 - } + }, + "alias": 0 } }, "metaData": [{ diff --git a/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_filter_test.json b/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_filter_test.json index c2d71b76ba90..2fdee59e51c8 100644 --- a/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_filter_test.json +++ b/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_filter_test.json @@ -52,8 +52,7 @@ "id": 0 }], "sampleRatio": 1.0 - }, - "alias": 0 + } } }, "metaData": [{ @@ -80,7 +79,8 @@ }] }] } - } + }, + "alias": -1 }] }, { "opr": { @@ -127,7 +127,8 @@ }] }, "sampleRatio": 1.0 - } + }, + "alias": 0 } }, "metaData": [{ diff --git a/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_test.json b/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_test.json index 73be464d1957..fa03b93fb621 100644 --- a/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_test.json +++ b/interactive_engine/compiler/src/test/resources/proto/partitioned_edge_expand_vertex_test.json @@ -52,7 +52,8 @@ "id": 0 }], "sampleRatio": 1.0 - } + }, + "alias": 0 } }, "metaData": [{ @@ -79,8 +80,7 @@ }] }] } - }, - "alias": -1 + } }] }, { "opr": {