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

[GAIA Compiler] Build algebra layer structures for filter and project in GraphBuilder #2482

Merged
merged 42 commits into from Mar 10, 2023

Conversation

shirly121
Copy link
Collaborator

What do these changes do?

Related issue number

Fixes

shirly121 and others added 30 commits November 4, 2022 16:45
@codecov-commenter
Copy link

codecov-commenter commented Mar 6, 2023

Codecov Report

Merging #2482 (85819bc) into main (a012a92) will decrease coverage by 32.76%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2482       +/-   ##
===========================================
- Coverage   72.62%   39.87%   -32.76%     
===========================================
  Files          88       88               
  Lines        9816     9816               
===========================================
- Hits         7129     3914     -3215     
- Misses       2687     5902     +3215     

see 47 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a012a92...85819bc. Read the comment docs.

}

/**
* 10 > 20 -> always returns false, create {@link LogicalValues} which carries all data types of the node before the filter
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the behavior?

.variable("a", "age");
Project project = (Project) builder.project(ImmutableList.of(variable)).build();
Assert.assertEquals("[a.age]", project.getProjects().toString());
Assert.assertEquals("RecordType(INTEGER age)", project.getRowType().toString());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are we getting the DataType of age?

Copy link
Collaborator Author

@shirly121 shirly121 Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

project.getRowType()返回所有expressions的类型信息,是一个RelRecord(List<RelDataTypeField>)类型,每一个expression都一一对应一个RelDataTypeFieldRelDataTypeField.getType()就能拿到对应expression的类型;

project.getRowType().toString());
}

// project("a.age") -> expr: "a.age", alias: "age"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why alias: age ?

Copy link
Collaborator Author

@shirly121 shirly121 Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

project/aggregate都使用了calcite的推断方式:

  1. 如果expression是variable类型,则尽量使用variable中的tag/property来命名,i.e. a->a, a.age -> age ;
  2. 对于其他的expression,则通过$f前缀加上id来命名,其中id是从0开始直到推断出一个不重复的命名为止 ;

这里是推断的主要逻辑:https://github.com/shirly121/GraphScope/blob/013f8d69492ae18ac524173aa3c37fb9d31b5619/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/AliasInference.java#L77

Assert.assertEquals("RecordType(INTEGER age)", project.getRowType().toString());
}

// project("a.age+1") -> expr: "a.age+1", alias: "$f0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is $f0 standing for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上

@@ -93,6 +97,27 @@ public RelNode getGetV() {
return getV;
}

@Override
public String toString() {
return "PathExpandConfig{"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always use StringBuilder for such long string concatenation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

public static GraphLogicalExpand create(
GraphOptCluster cluster, List<RelHint> hints, RelNode input, TableConfig tableConfig) {
return new GraphLogicalExpand(cluster, hints, input, tableConfig);
}

private GraphOpt.Expand directionOpt() {
public GraphOpt.Expand getOpt() {
ObjectUtils.requireNonEmpty(hints);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the behavior of requireNonEmpty? Throwing error, or quick the program. How does it affect the working thread?

Copy link
Collaborator Author

@shirly121 shirly121 Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

只是抛出异常,当前线程返回一个异常信息,不会影响整个服务

}

public static GraphLogicalGetV create(
GraphOptCluster cluster, List<RelHint> hints, RelNode input, TableConfig tableConfig) {
return new GraphLogicalGetV(cluster, hints, input, tableConfig);
}

private GraphOpt.GetV getVOpt() {
public GraphOpt.GetV getOpt() {
ObjectUtils.requireNonEmpty(hints);
RelHint optHint = hints.get(0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is hints?

Copy link
Collaborator Author

@shirly121 shirly121 Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List是用来额外传算子配置的config,类似于kv键值对;点/边/path_expand的opt都是通过kv给到对应算子,再通过getOpt函数取出来;

这里是GraphBuilder创建图算子时设置的地方:https://github.com/shirly121/GraphScope/blob/ir_algebra_tmp/interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphBuilder.java#L239

Copy link
Collaborator

@longbinlai longbinlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

/**
* each element type of path expand
*/
public class GraphPxdElementType extends AbstractSqlType {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use GraphPathExpandElementType. The short name is not very clear.

}

public enum Opt {
ID,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment these options.

@longbinlai longbinlai merged commit cf5d75e into alibaba:main Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants