From 54a22fb65849e2f9341a8d1551b241d6642bb946 Mon Sep 17 00:00:00 2001 From: Xiaoli Zhou Date: Mon, 6 Jan 2025 19:10:26 +0800 Subject: [PATCH] fix(interactive): Fix Bugs of Alias Id in `Union` (#4404) ## What do these changes do? as titled. ## Related issue number Fixes #4385 --- .../common/exception/FrontendException.java | 2 +- .../common/ir/type/GraphTypeFactoryImpl.java | 43 ++++++++++++++++++- .../gremlin/antlr4x/GraphBuilderTest.java | 34 +++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/exception/FrontendException.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/exception/FrontendException.java index 2831e8a7ccbf..2de97eb596dd 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/exception/FrontendException.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/exception/FrontendException.java @@ -65,7 +65,7 @@ public String getMessage() { StringBuilder sb = new StringBuilder(); sb.append("ErrorCode: ").append(errorCode.name()).append("\n"); String msg = super.getMessage(); - if (!msg.endsWith("\n")) { + if (msg == null || !msg.endsWith("\n")) { msg += "\n"; } sb.append("Message: ").append(msg); diff --git a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java index fdb6aa9e7e55..881e8a6837fa 100644 --- a/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java +++ b/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphTypeFactoryImpl.java @@ -22,8 +22,9 @@ import com.google.common.collect.Maps; import org.apache.calcite.jdbc.JavaTypeFactoryImpl; -import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.*; import org.apache.calcite.rex.RexNode; +import org.apache.calcite.util.Util; import org.checkerframework.checker.nullness.qual.Nullable; import java.nio.charset.Charset; @@ -100,6 +101,46 @@ public RelDataType createArbitraryMapType( return super.leastRestrictive(types); } + /** + * reimplement the {@link RelDataTypeFactoryImpl#leastRestrictiveStructuredType(List)} method + * to maintain the original alias id in the input types + * @param types + * @return + */ + @Override + protected @Nullable RelDataType leastRestrictiveStructuredType(final List types) { + final RelDataType type0 = types.get(0); + if (!type0.isStruct()) { + return null; + } + final int fieldCount = type0.getFieldCount(); + boolean isNullable = false; + for (RelDataType type : types) { + if (!type.isStruct()) { + return null; + } + if (type.getFieldList().size() != fieldCount) { + return null; + } + isNullable |= type.isNullable(); + } + List fields = Lists.newArrayList(); + for (int j = 0; j < fieldCount; ++j) { + final int k = j; + RelDataType type = + leastRestrictive(Util.transform(types, t -> t.getFieldList().get(k).getType())); + if (type == null) { + return null; + } + fields.add( + new RelDataTypeFieldImpl( + type0.getFieldList().get(j).getName(), + type0.getFieldList().get(j).getIndex(), + type)); + } + return new RelRecordType(StructKind.FULLY_QUALIFIED, fields, isNullable); + } + // re-implement lease-restrictive type inference for arbitrary map types // for each key type and value type, check if they have a least-restrictive type, otherwise // return null diff --git a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java index 97279d78a2da..539212fa6a13 100644 --- a/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java +++ b/interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java @@ -23,6 +23,8 @@ 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.rel.GraphLogicalProject; +import com.alibaba.graphscope.common.ir.rex.RexGraphVariable; 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; @@ -1852,4 +1854,36 @@ public void g_V_match_as_a_person_both_as_b_test() { + " person]}], alias=[a], opt=[VERTEX])", after3.explain().trim()); } + + @Test + public void union_valueMap_test() { + Configs configs = + new Configs( + ImmutableMap.of( + "graph.planner.is.on", + "true", + "graph.planner.opt", + "CBO", + "graph.planner.rules", + "FilterIntoJoinRule, FilterMatchRule, ExtendIntersectRule," + + " ExpandGetVFusionRule")); + GraphRelOptimizer optimizer = new GraphRelOptimizer(configs); + IrMeta irMeta = + Utils.mockIrMeta( + "schema/ldbc.json", + "statistics/ldbc30_statistics.json", + optimizer.getGlogueHolder()); + GraphBuilder builder = Utils.mockGraphBuilder(optimizer, irMeta); + RelNode node1 = + eval( + "g.V().hasLabel(\"PERSON\").has(\"id\"," + + " 1816).union(outE(\"LIKES\").limit(10).as('a').inV().as('b').select('a','b').by(valueMap(\"creationDate\"))," + + " outE(\"KNOWS\").limit(10).as('c').inV().as('d').select('c','d').by(valueMap(\"creationDate\")))", + builder); + RelNode after1 = optimizer.optimize(node1, new GraphIOProcessor(builder, irMeta)); + GraphLogicalProject project = (GraphLogicalProject) after1; + RexNode expr = project.getProjects().get(0); + RexGraphVariable var = (RexGraphVariable) expr; + Assert.assertEquals(2, var.getAliasId()); + } }