Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(interactive): Fix Bugs in Converting Match into Join #3311

Merged
merged 2 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -306,13 +306,12 @@
RelNode input = size() > 0 ? peek() : null;
RelNode match =
GraphLogicalMultiMatch.create(
(GraphOptCluster) cluster,
null,
input,
first,
ImmutableList.copyOf(others));
if (size() > 0) pop();
push(match);
(GraphOptCluster) cluster, null, null, first, ImmutableList.copyOf(others));
if (input == null) {
push(match);
} else {
push(match).join(getJoinRelType(GraphOpt.Match.INNER), getJoinCondition(input, match));
}
return this;
}

Expand Down Expand Up @@ -642,25 +641,25 @@
}
}

private boolean isCurrentSupported(SqlOperator operator) {
SqlKind sqlKind = operator.getKind();
return sqlKind.belongsTo(SqlKind.BINARY_ARITHMETIC)
|| sqlKind.belongsTo(SqlKind.COMPARISON)
|| sqlKind == SqlKind.AND
|| sqlKind == SqlKind.OR
|| sqlKind == SqlKind.DESCENDING
|| (sqlKind == SqlKind.OTHER_FUNCTION && operator.getName().equals("POWER"))
|| (sqlKind == SqlKind.MINUS_PREFIX)
|| (sqlKind == SqlKind.CASE)
|| (sqlKind == SqlKind.PROCEDURE_CALL)
|| (sqlKind == SqlKind.NOT)
|| sqlKind == SqlKind.ARRAY_VALUE_CONSTRUCTOR
|| sqlKind == SqlKind.IS_NULL
|| sqlKind == SqlKind.IS_NOT_NULL
|| sqlKind == SqlKind.EXTRACT
|| sqlKind == SqlKind.SEARCH;
}

Check notice on line 662 in interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java

View check run for this annotation

codefactor.io / CodeFactor

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java#L644-L662

Complex Method
@Override
public GraphBuilder filter(RexNode... conditions) {
return filter(ImmutableList.copyOf(conditions));
Expand Down Expand Up @@ -707,57 +706,57 @@
return builder;
}

private AbstractBindableTableScan fuseFilters(
AbstractBindableTableScan tableScan, RexNode condition) {
List<Comparable> labelValues = Lists.newArrayList();
List<RexNode> uniqueKeyFilters = Lists.newArrayList();
List<RexNode> extraFilters = Lists.newArrayList();
classifyFilters(tableScan, condition, labelValues, uniqueKeyFilters, extraFilters);
if (!labelValues.isEmpty()) {
GraphLabelType labelType =
((GraphSchemaType) tableScan.getRowType().getFieldList().get(0).getType())
.getLabelType();
List<String> labelsToKeep =
labelType.getLabelsEntry().stream()
.filter(k -> labelValues.contains(k.getLabel()))
.map(k -> k.getLabel())
.collect(Collectors.toList());
Preconditions.checkArgument(
!labelsToKeep.isEmpty(),
"cannot find common labels between values= " + labelValues + " and label=",
labelType);
if (labelsToKeep.size() < labelType.getLabelsEntry().size()) {
GraphBuilder builder =
(GraphBuilder)
GraphPlanner.relBuilderFactory.create(
getCluster(), getRelOptSchema());
LabelConfig newLabelConfig = new LabelConfig(false);
labelsToKeep.forEach(k -> newLabelConfig.addLabel(k));
if (tableScan instanceof GraphLogicalSource) {
builder.source(
new SourceConfig(
((GraphLogicalSource) tableScan).getOpt(),
newLabelConfig,
tableScan.getAliasName()));
} else if (tableScan instanceof GraphLogicalExpand) {
((GraphBuilder) builder.push(tableScan.getInput(0)))
.expand(
new ExpandConfig(
((GraphLogicalExpand) tableScan).getOpt(),
newLabelConfig,
tableScan.getAliasName()));
} else if (tableScan instanceof GraphLogicalGetV) {
((GraphBuilder) builder.push(tableScan.getInput(0)))
.getV(
new GetVConfig(
((GraphLogicalGetV) tableScan).getOpt(),
newLabelConfig,
tableScan.getAliasName()));
}
if (builder.size() > 0) {
// check if the property still exist after updating the label type
RexVisitor propertyChecker =
new RexVisitorImpl<Void>(true) {

Check notice on line 759 in interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java

View check run for this annotation

codefactor.io / CodeFactor

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java#L709-L759

Complex Method
@Override
public Void visitInputRef(RexInputRef inputRef) {
if (inputRef instanceof RexGraphVariable) {
Expand Down Expand Up @@ -1197,102 +1196,102 @@
* @param nodes build limit() if empty
* @return
*/
@Override
public GraphBuilder sortLimit(
@Nullable RexNode offsetNode,
@Nullable RexNode fetchNode,
Iterable<? extends RexNode> nodes) {
if (offsetNode != null && !(offsetNode instanceof RexLiteral)) {
throw new IllegalArgumentException("OFFSET node must be RexLiteral");
}
if (offsetNode != null && !(offsetNode instanceof RexLiteral)) {
throw new IllegalArgumentException("FETCH node must be RexLiteral");
}

RelNode input = requireNonNull(peek(), "frame stack is empty");

List<RelDataTypeField> originalFields = input.getRowType().getFieldList();

Registrar registrar = new Registrar(this, input, true);
List<RexNode> registerNodes = registrar.registerExpressions(ImmutableList.copyOf(nodes));

// expressions need to be projected in advance
if (!registrar.getExtraNodes().isEmpty()) {
project(registrar.getExtraNodes(), registrar.getExtraAliases(), registrar.isAppend());
RexTmpVariableConverter converter = new RexTmpVariableConverter(true, this);
registerNodes =
registerNodes.stream()
.map(k -> k.accept(converter))
.collect(Collectors.toList());
input = requireNonNull(peek(), "frame stack is empty");
}

List<RelFieldCollation> fieldCollations = fieldCollations(registerNodes);
Config config = Utils.getFieldValue(RelBuilder.class, this, "config");

// limit 0 -> return empty value
if ((fetchNode != null && RexLiteral.intValue(fetchNode) == 0) && config.simplifyLimit()) {
return (GraphBuilder) empty();
}

// output all results without any order -> skip
if (offsetNode == null && fetchNode == null && fieldCollations.isEmpty()) {
return this; // sort is trivial
}
// sortLimit is actually limit if collations are empty
if (fieldCollations.isEmpty()) {
// fuse limit with the previous sort operator
// order + limit -> topK
if (input instanceof Sort) {
Sort sort2 = (Sort) input;
// output all results without any limitations
if (sort2.offset == null && sort2.fetch == null) {
RelNode sort =
GraphLogicalSort.create(
sort2.getInput(), sort2.collation, offsetNode, fetchNode);
replaceTop(sort);
return this;
}
}
// order + project + limit -> topK + project
if (input instanceof Project) {
Project project = (Project) input;
if (project.getInput() instanceof Sort) {
Sort sort2 = (Sort) project.getInput();
if (sort2.offset == null && sort2.fetch == null) {
RelNode sort =
GraphLogicalSort.create(
sort2.getInput(), sort2.collation, offsetNode, fetchNode);
replaceTop(
GraphLogicalProject.create(
(GraphOptCluster) project.getCluster(),
project.getHints(),
sort,
project.getProjects(),
project.getRowType(),
((GraphLogicalProject) project).isAppend()));
return this;
}
}
}
}
RelNode sort =
GraphLogicalSort.create(
input, GraphRelCollations.of(fieldCollations), offsetNode, fetchNode);
replaceTop(sort);
// to remove the extra columns we have added
if (!registrar.getExtraAliases().isEmpty()) {
List<RexNode> originalExprs = new ArrayList<>();
List<String> originalAliases = new ArrayList<>();
for (RelDataTypeField field : originalFields) {
originalExprs.add(variable(field.getName()));
originalAliases.add(field.getName());
}
project(originalExprs, originalAliases, false);
}
return this;
}

Check notice on line 1294 in interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java

View check run for this annotation

codefactor.io / CodeFactor

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java#L1199-L1294

Complex Method
@Override
public RelBuilder join(
JoinRelType joinType, RexNode condition, Set<CorrelationId> variablesSet) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.alibaba.graphscope.cypher.antlr4.visitor.type.ExprVisitorResult;
import com.alibaba.graphscope.grammar.CypherGSBaseVisitor;
import com.alibaba.graphscope.grammar.CypherGSParser;
import com.google.common.base.Preconditions;

import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rex.RexCall;
Expand Down Expand Up @@ -63,6 +64,8 @@ public GraphBuilder visitOC_Match(CypherGSParser.OC_MatchContext ctx) {
sentences.get(0),
(ctx.OPTIONAL() != null) ? GraphOpt.Match.OPTIONAL : GraphOpt.Match.INNER);
} else if (sentences.size() > 1) {
Preconditions.checkArgument(
ctx.OPTIONAL() == null, "multiple sentences in match should not be optional");
builder.match(sentences.get(0), sentences.subList(1, sentences.size()));
} else {
throw new IllegalArgumentException("sentences in match should not be empty");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,4 +311,39 @@ public void match_16_test() {
+ "], matchOpt=[INNER])",
node.explain().trim());
}

@Test
public void match_17_test() {
RelNode node =
Utils.eval(
"Match (a:person)-[]->(b:person) Match (a:person)-[]-(c:person),"
+ " (c:person)-[]->(b:person) Return a, b")
.build();
Assert.assertEquals(
"GraphLogicalProject(a=[a], b=[b], isAppend=[false])\n"
+ " LogicalJoin(condition=[AND(=(a, a), =(b, b))], joinType=[inner])\n"
+ " GraphLogicalSingleMatch(input=[null],"
+ " sentence=[GraphLogicalGetV(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[b], opt=[END])\n"
+ " GraphLogicalExpand(tableConfig=[{isAll=true, tables=[created, knows]}],"
+ " alias=[DEFAULT], opt=[OUT])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], opt=[VERTEX])\n"
+ "], matchOpt=[INNER])\n"
+ " GraphLogicalMultiMatch(input=[null],"
+ " sentences=[{s0=[GraphLogicalGetV(tableConfig=[{isAll=false,"
+ " tables=[person]}], alias=[c], opt=[OTHER])\n"
+ " GraphLogicalExpand(tableConfig=[{isAll=true, tables=[created, knows]}],"
+ " alias=[DEFAULT], opt=[BOTH])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[a], opt=[VERTEX])\n"
+ "], s1=[GraphLogicalGetV(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[b], opt=[END])\n"
+ " GraphLogicalExpand(tableConfig=[{isAll=true, tables=[created, knows]}],"
+ " alias=[DEFAULT], opt=[OUT])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[c], opt=[VERTEX])\n"
+ "]}])",
node.explain().trim());
}
}
Loading