Skip to content

Commit

Permalink
fix(interactive): Fix Bugs of Type Inference in PathExpand (#3837)
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?
as titled

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

## Related issue number

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

Fixes #3791

---------

Co-authored-by: 霄磊 <xiaolei.zl@alibaba-inc.com>
Co-authored-by: bingqing.lbq <bingqing.lbq@alibaba-inc.com>
  • Loading branch information
@shirly121 @zhanglei1949 @BingqingLyu
3 people authored May 30, 2024
1 parent b884c56 commit ceb1421
Show file tree
Hide file tree
Showing 12 changed files with 476 additions and 120 deletions.
43 changes: 31 additions & 12 deletions ...ine/compiler/src/main/java/com/alibaba/graphscope/common/ir/planner/GraphIOProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.alibaba.graphscope.common.ir.tools.Utils;
import com.alibaba.graphscope.common.ir.tools.config.*;
import com.alibaba.graphscope.common.ir.type.GraphLabelType;
import com.alibaba.graphscope.common.ir.type.GraphPathType;
import com.alibaba.graphscope.common.ir.type.GraphSchemaType;
import com.alibaba.graphscope.common.store.IrMeta;
import com.alibaba.graphscope.groot.common.schema.api.EdgeRelation;
Expand Down Expand Up @@ -340,10 +341,21 @@ private PatternEdge visitAndAddPxdEdge(
: ((RexLiteral) pxd.getFetch())
.getValueAs(Number.class)
.intValue();
GraphPathType pathType =
(GraphPathType)
pxd.getRowType().getFieldList().get(0).getType();
GraphLabelType labelType =
((GraphSchemaType) pathType.getComponentType().getGetVType())
.getLabelType();
List<Integer> innerGetVTypes =
labelType.getLabelsEntry().stream()
.map(k -> k.getLabelId())
.collect(Collectors.toList());
ElementDetails newDetails =
new ElementDetails(
expandEdge.getElementDetails().getSelectivity(),
new PathExpandRange(offset, fetch));
new PathExpandRange(offset, fetch),
innerGetVTypes);
expandEdge =
(expandEdge instanceof SinglePatternEdge)
? new SinglePatternEdge(
Expand Down Expand Up @@ -442,16 +454,18 @@ private void checkPattern(Pattern pattern) {
expectedDstIds.add(k.getSrcLabelId());
}
});
Preconditions.checkArgument(
Sets.newHashSet(src.getVertexTypeIds()).equals(expectedSrcIds),
"src vertex types %s not consistent with edge types %s",
src.getVertexTypeIds(),
edge.getEdgeTypeIds());
Preconditions.checkArgument(
Sets.newHashSet(dst.getVertexTypeIds()).equals(expectedDstIds),
"dst vertex types %s not consistent with edge types %s",
dst.getVertexTypeIds(),
edge.getEdgeTypeIds());
if (edge.getElementDetails().getRange() == null) {
Preconditions.checkArgument(
Sets.newHashSet(src.getVertexTypeIds()).equals(expectedSrcIds),
"src vertex types %s not consistent with edge types %s",
src.getVertexTypeIds(),
edge.getEdgeTypeIds());
Preconditions.checkArgument(
Sets.newHashSet(dst.getVertexTypeIds()).equals(expectedDstIds),
"dst vertex types %s not consistent with edge types %s",
dst.getVertexTypeIds(),
edge.getEdgeTypeIds());
}
}
}
}
Expand Down Expand Up @@ -776,8 +790,13 @@ private RelNode createExpandGetV(
if (edgeValue.getFilter() != null) {
pxdBuilder.filter(edgeValue.getFilter());
}
GetVConfig innerGetVConfig =
new GetVConfig(
getVConfig.getOpt(),
createLabels(edge.getElementDetails().getPxdInnerGetVTypes(), true),
getVConfig.getAlias());
pxdBuilder
.getV(getVConfig)
.getV(innerGetVConfig)
.resultOpt(GraphOpt.PathExpandResult.END_V)
.pathOpt(GraphOpt.PathExpandPath.ARBITRARY)
.alias(edgeValue.getAlias())
Expand Down
5 changes: 4 additions & 1 deletion ...r/src/main/java/com/alibaba/graphscope/common/ir/planner/rules/JoinDecompositionRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,10 @@ private PatternEdge createNewEdge(
// details.
int newEdgeId = oldEdge.getId();
ElementDetails newDetails =
new ElementDetails(oldEdge.getElementDetails().getSelectivity(), newRange);
new ElementDetails(
oldEdge.getElementDetails().getSelectivity(),
newRange,
oldEdge.getElementDetails().getPxdInnerGetVTypes());
return (oldEdge instanceof SinglePatternEdge)
? new SinglePatternEdge(
newSrc,
Expand Down
41 changes: 20 additions & 21 deletions ...iler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphLogicalPathExpand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.apache.calcite.rel.type.RelRecordType;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.SqlExplainLevel;
import org.apache.commons.lang3.ObjectUtils;
import org.checkerframework.checker.nullness.qual.Nullable;

import java.util.List;
Expand Down Expand Up @@ -318,27 +317,27 @@ protected RelDataType deriveRowType() {
}

private GraphPathType.ElementType getElementType() {
switch (resultOpt) {
case ALL_V:
case END_V:
RelNode getV = this.fused != null ? this.fused : this.getV;
ObjectUtils.requireNonEmpty(
getV.getRowType().getFieldList(),
"data type of getV operator should have at least one column field");
return new GraphPathType.ElementType(
getV.getRowType().getFieldList().get(0).getType());
case ALL_V_E:
default:
ObjectUtils.requireNonEmpty(
this.expand.getRowType().getFieldList(),
"data type of expand operator should have at least one column field");
ObjectUtils.requireNonEmpty(
this.getV.getRowType().getFieldList(),
"data type of getV operator should have at least one column field");
return new GraphPathType.ElementType(
this.expand.getRowType().getFieldList().get(0).getType(),
this.getV.getRowType().getFieldList().get(0).getType());
GraphLogicalExpand innerExpand = null;
GraphLogicalGetV innerGetV = null;
if (this.fused instanceof GraphPhysicalGetV) {
innerExpand = ((GraphPhysicalExpand) this.fused.getInput(0)).getFusedExpand();
innerGetV = ((GraphPhysicalGetV) this.fused).getFusedGetV();
} else if (this.fused instanceof GraphPhysicalExpand) {
innerExpand = ((GraphPhysicalExpand) this.fused).getFusedExpand();
innerGetV = ((GraphPhysicalExpand) this.fused).getFusedGetV();
} else if (this.expand != null && this.getV != null) {
innerExpand = (GraphLogicalExpand) this.expand;
innerGetV = (GraphLogicalGetV) this.getV;
}
Preconditions.checkArgument(
innerExpand != null && !innerExpand.getRowType().getFieldList().isEmpty(),
"data type of expand operator should have at least one column field");
Preconditions.checkArgument(
innerGetV != null && !innerGetV.getRowType().getFieldList().isEmpty(),
"data type of getV operator should have at least one column field");
return new GraphPathType.ElementType(
innerExpand.getRowType().getFieldList().get(0).getType(),
innerGetV.getRowType().getFieldList().get(0).getType());
}

@Override
Expand Down
4 changes: 4 additions & 0 deletions ...ompiler/src/main/java/com/alibaba/graphscope/common/ir/rel/graph/GraphPhysicalExpand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,8 @@ public RelNode accept(RelShuttle shuttle) {
}
return shuttle.visit(this);
}

public GraphLogicalGetV getFusedGetV() {
return fusedGetV;
}
}
30 changes: 25 additions & 5 deletions ...ain/java/com/alibaba/graphscope/common/ir/rel/metadata/glogue/pattern/ElementDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,47 @@

package com.alibaba.graphscope.common.ir.rel.metadata.glogue.pattern;

import com.google.common.collect.ImmutableList;

import org.checkerframework.checker.nullness.qual.Nullable;

import java.util.Collections;
import java.util.List;
import java.util.Objects;

public class ElementDetails implements Comparable<ElementDetails> {
private final double selectivity;
// the range is not null if and only if the element denotes a path expand operator
private final @Nullable PathExpandRange range;
// record inner getV types of path expand
private final List<Integer> pxdInnerGetVTypes;
private boolean optional;

public ElementDetails() {
this(1.0d);
}

public ElementDetails(double selectivity) {
this(selectivity, null);
this(selectivity, null, ImmutableList.of());
}

public ElementDetails(double selectivity, @Nullable PathExpandRange range) {
this(selectivity, range, false);
public ElementDetails(
double selectivity, @Nullable PathExpandRange range, List<Integer> pxdInnerGetVTypes) {
this(selectivity, range, pxdInnerGetVTypes, false);
}

public ElementDetails(double selectivity, boolean optional) {
this(selectivity, null, optional);
this(selectivity, null, ImmutableList.of(), optional);
}

public ElementDetails(double selectivity, @Nullable PathExpandRange range, boolean optional) {
public ElementDetails(
double selectivity,
@Nullable PathExpandRange range,
List<Integer> pxdInnerVertexTypes,
boolean optional) {
this.selectivity = selectivity;
this.range = range;
this.pxdInnerGetVTypes = pxdInnerVertexTypes;
this.optional = optional;
}

Expand All @@ -55,6 +67,7 @@ public boolean equals(Object o) {
ElementDetails details = (ElementDetails) o;
return Double.compare(details.selectivity, selectivity) == 0
&& Objects.equals(range, details.range)
&& Objects.equals(pxdInnerGetVTypes, details.pxdInnerGetVTypes)
&& optional == details.optional;
}

Expand All @@ -79,6 +92,10 @@ public void setOptional(boolean optional) {
this.optional = optional;
}

public List<Integer> getPxdInnerGetVTypes() {
return Collections.unmodifiableList(this.pxdInnerGetVTypes);
}

@Override
public int compareTo(ElementDetails o) {
int compare = Double.compare(this.selectivity, o.selectivity);
Expand All @@ -89,6 +106,9 @@ public int compareTo(ElementDetails o) {
if (compare != 0) {
return compare;
}
if (!this.pxdInnerGetVTypes.equals(o.pxdInnerGetVTypes)) {
return -1;
}
if (this.range != null && o.range != null) {
return this.range.compareTo(o.range);
}
Expand Down
4 changes: 1 addition & 3 deletions ...ve_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/GraphPathType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
import org.apache.calcite.sql.type.SqlTypeName;
import org.checkerframework.checker.nullness.qual.Nullable;

import java.util.Objects;

public class GraphPathType extends ArraySqlType {
public GraphPathType(ElementType elementType) {
this(elementType, false);
Expand Down Expand Up @@ -52,7 +50,7 @@ public ElementType(RelDataType getVType) {
public ElementType(RelDataType expandType, RelDataType getVType) {
super(SqlTypeName.OTHER, false, null);
this.expandType = expandType;
this.getVType = Objects.requireNonNull(getVType);
this.getVType = getVType;
computeDigest();
}

Expand Down
Loading

0 comments on commit ceb1421

Please sign in to comment.