fix(interactive): Fix Bugs of GOpt Caused By Multiple Filters of Sing…

…le Vertex (#3831) Fixes #3791 --------- Co-authored-by: Longbin Lai <longbin.lailb@alibaba-inc.com>
alibaba · Jun 4, 2024 · cbc2691 · cbc2691
1 parent bae1819
commit cbc2691
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 10 deletions.
diff --git a/...ine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/GraphIOProcessor.java b/...ine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/GraphIOProcessor.java
@@ -48,7 +48,6 @@
 
 import org.apache.calcite.plan.GraphOptCluster;
 import org.apache.calcite.plan.RelOptTable;
-import org.apache.calcite.plan.RelOptUtil;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.RelVisitor;
 import org.apache.calcite.rel.core.JoinRelType;
@@ -267,15 +266,20 @@ private PatternVertex visitAndAddVertex(
                                 vertexOrEdgeDetails.put(existVertex, new DataValue(alias, filters));
                             } else if (filters != null) {
                                 DataValue value = vertexOrEdgeDetails.get(existVertex);
-                                if (value.getFilter() == null
-                                        || !RelOptUtil.conjunctions(value.getFilter())
-                                                .containsAll(RelOptUtil.conjunctions(filters))) {
-                                    throw new IllegalArgumentException(
-                                            "filters "
-                                                    + filters
-                                                    + " not exist in the previous vertex filters "
-                                                    + value.getFilter());
-                                }
+                                // reset condition
+                                RexNode newCondition =
+                                        (value.getFilter() == null)
+                                                ? filters
+                                                : RexUtil.composeConjunction(
+                                                        builder.getRexBuilder(),
+                                                        ImmutableList.of(
+                                                                filters, value.getFilter()));
+                                vertexOrEdgeDetails.put(
+                                        existVertex,
+                                        new DataValue(
+                                                value.getAlias(),
+                                                newCondition,
+                                                value.getParentAlias()));
                             }
                             return existVertex;
                         }

diff --git a/...ngine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java b/...ngine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java
@@ -16,19 +16,24 @@
 
 package com.alibaba.graphscope.gremlin.antlr4x;
 
+import com.alibaba.graphscope.common.config.Configs;
 import com.alibaba.graphscope.common.ir.Utils;
+import com.alibaba.graphscope.common.ir.planner.GraphIOProcessor;
+import com.alibaba.graphscope.common.ir.planner.GraphRelOptimizer;
 import com.alibaba.graphscope.common.ir.planner.rules.ExpandGetVFusionRule;
 import com.alibaba.graphscope.common.ir.runtime.proto.RexToProtoConverter;
 import com.alibaba.graphscope.common.ir.tools.GraphBuilder;
 import com.alibaba.graphscope.common.ir.tools.GraphStdOperatorTable;
 import com.alibaba.graphscope.common.ir.tools.config.GraphOpt;
 import com.alibaba.graphscope.common.ir.tools.config.SourceConfig;
 import com.alibaba.graphscope.common.ir.type.GraphProperty;
+import com.alibaba.graphscope.common.store.IrMeta;
 import com.alibaba.graphscope.common.utils.FileUtils;
 import com.alibaba.graphscope.gaia.proto.OuterExpression;
 import com.alibaba.graphscope.gremlin.antlr4x.parser.GremlinAntlr4Parser;
 import com.alibaba.graphscope.gremlin.antlr4x.visitor.GraphBuilderVisitor;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.protobuf.util.JsonFormat;
 
 import org.antlr.v4.runtime.tree.ParseTree;
@@ -37,16 +42,45 @@
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexNode;
 import org.junit.Assert;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 public class GraphBuilderTest {
+    private static Configs configs;
+    private static IrMeta irMeta;
+    private static GraphRelOptimizer optimizer;
+
+    @BeforeClass
+    public static void beforeClass() {
+        configs =
+                new Configs(
+                        ImmutableMap.of(
+                                "graph.planner.is.on",
+                                "true",
+                                "graph.planner.opt",
+                                "CBO",
+                                "graph.planner.rules",
+                                "FilterIntoJoinRule, FilterMatchRule, ExtendIntersectRule,"
+                                        + " ExpandGetVFusionRule",
+                                "graph.planner.cbo.glogue.schema",
+                                "target/test-classes/statistics/modern_statistics.txt"));
+        optimizer = new GraphRelOptimizer(configs);
+        irMeta = Utils.mockSchemaMeta("schema/modern.json");
+    }
+
     public static RelNode eval(String query) {
         GraphBuilder builder = Utils.mockGraphBuilder();
         GraphBuilderVisitor visitor = new GraphBuilderVisitor(builder);
         ParseTree parseTree = new GremlinAntlr4Parser().parse(query);
         return visitor.visit(parseTree).build();
     }
 
+    public static RelNode eval(String query, GraphBuilder builder) {
+        GraphBuilderVisitor visitor = new GraphBuilderVisitor(builder);
+        ParseTree parseTree = new GremlinAntlr4Parser().parse(query);
+        return visitor.visit(parseTree).build();
+    }
+
     @Test
     public void g_V_test() {
         RelNode node = eval("g.V()");
@@ -1596,6 +1630,31 @@ public void g_V_match_as_a_out_as_b_count_test() {
                 node.explain().trim());
     }
 
+    // the filter conditions `has('age', 10)` and `has("name", "male")` should be composed and fused
+    // into the person 'b'
+    @Test
+    public void g_V_match_as_a_out_as_b_hasLabel_has_out_as_c_count_test() {
+        GraphBuilder builder = Utils.mockGraphBuilder(optimizer, irMeta);
+        RelNode node =
+                eval(
+                        "g.V().match(__.as(\"a\").out(\"knows\").as(\"b\").has('age', 10),"
+                                + " __.as(\"b\").hasLabel(\"person\").has(\"name\","
+                                + " \"male\").out(\"knows\").as(\"c\")).count()",
+                        builder);
+        RelNode after = optimizer.optimize(node, new GraphIOProcessor(builder, irMeta));
+        Assert.assertEquals(
+                "GraphLogicalAggregate(keys=[{variables=[], aliases=[]}], values=[[{operands=[a, b,"
+                        + " c], aggFunction=COUNT, alias='$f0', distinct=false}]])\n"
+                        + "  GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}],"
+                        + " alias=[a], startAlias=[b], opt=[IN], physicalOpt=[VERTEX])\n"
+                        + "    GraphPhysicalExpand(tableConfig=[{isAll=false, tables=[knows]}],"
+                        + " alias=[c], startAlias=[b], opt=[OUT], physicalOpt=[VERTEX])\n"
+                        + "      GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+                        + " alias=[b], fusedFilter=[[AND(=(_.name, _UTF-8'male'), =(_.age, 10))]],"
+                        + " opt=[VERTEX])",
+                after.explain().trim());
+    }
+
     // id is the primary key of label 'person', should be fused as 'uniqueKeyFilters'
     @Test
     public void g_V_has_label_person_id_test() {