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 expressions #2461

Merged
merged 5 commits into from Feb 27, 2023

Conversation

shirly121
Copy link
Collaborator

@shirly121 shirly121 commented Feb 24, 2023

What do these changes do?

  1. implement variable(..) in GraphBuilder.java to build algebra layer structures for expressions.
  2. add NameOrId to denote property key as name or id in RexGraphVariable.

Related issue number

Fixes

} catch (IllegalArgumentException e) {
return;
}
Assert.fail("tag 'a' should not exist");
Copy link
Collaborator

Choose a reason for hiding this comment

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

tag 'a' does not exist is a better description.

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 void equal_plus_test() {
RexNode var = builder.source(mockSourceConfig("a")).variable("a", "age");
RexNode plus = builder.call(GraphStdOperatorTable.PLUS, var, builder.literal(10));
RexNode equal = builder.call(GraphStdOperatorTable.EQUALS, plus, builder.literal(30));
Copy link
Collaborator

Choose a reason for hiding this comment

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

According to your comment, this should be GreaterThan ?

Copy link
Collaborator Author

@shirly121 shirly121 Feb 27, 2023

Choose a reason for hiding this comment

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

the comment is fixed

@@ -76,7 +76,7 @@
"comment": "name"
},
{
"id": 3,
"id": 4,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why these ids changed?

@codecov-commenter
Copy link

Codecov Report

Merging #2461 (d8443da) into main (e9850d3) will decrease coverage by 33.27%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2461       +/-   ##
===========================================
- Coverage   73.22%   39.95%   -33.27%     
===========================================
  Files          88       88               
  Lines        9769     9769               
===========================================
- Hits         7153     3903     -3250     
- Misses       2616     5866     +3250     
Impacted Files Coverage Δ
python/graphscope/tests/unittest/test_java_app.py 0.00% <0.00%> (-100.00%) ⬇️
...ython/graphscope/tests/unittest/test_cython_ast.py 0.00% <0.00%> (-100.00%) ⬇️
...ython/graphscope/tests/unittest/test_serailaize.py 0.00% <0.00%> (-100.00%) ⬇️
python/graphscope/analytical/udf/patch.py 3.47% <0.00%> (-96.53%) ⬇️
python/graphscope/tests/unittest/test_app.py 0.00% <0.00%> (-96.32%) ⬇️
python/graphscope/tests/unittest/test_lazy.py 0.00% <0.00%> (-96.22%) ⬇️
...thon/graphscope/tests/unittest/test_scalability.py 0.00% <0.00%> (-96.16%) ⬇️
...hon/graphscope/tests/unittest/test_create_graph.py 0.00% <0.00%> (-92.89%) ⬇️
python/graphscope/tests/unittest/test_graph.py 0.00% <0.00%> (-85.92%) ⬇️
python/graphscope/tests/unittest/test_context.py 0.00% <0.00%> (-81.36%) ⬇️
... and 37 more

Continue to review full report at Codecov.

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

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

@longbinlai longbinlai merged commit f85f8a5 into alibaba:main Feb 27, 2023
@shirly121 shirly121 deleted the ir_algebra_tmp branch March 3, 2023 12:14
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