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 group and order in GraphBuilder
#2508
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2508 +/- ##
=======================================
Coverage 39.87% 39.87%
=======================================
Files 88 88
Lines 9816 9816
=======================================
Hits 3914 3914
Misses 5902 5902 see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
private final List<@Nullable String> aliases; | ||
|
||
public GraphGroupKeys(List<RexNode> variables, List<@Nullable String> aliases) { | ||
this.variables = Objects.requireNonNull(variables); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这两个地方需要检查长度一样吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这两个地方需要检查长度一样吗?
不强制用户给一样长度,对于aliases.size() < variables.size()情况,会内部自动为最后的variables生成aliases
/** | ||
* implement interfaces of {@code AggCall} by default, these interfaces are useless to {@code GraphAggCall} | ||
*/ | ||
public abstract class AbstractAggCall implements RelBuilder.AggCall { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have no idea what AbstractAggCall is different from AggCall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have no idea what AbstractAggCall is different from AggCall.
all fixed in commit 5bdf044
import java.util.Objects; | ||
|
||
/** | ||
* maintain the necessary info to create a {@code RexGraphVariable} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please describe why this class is necessary
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
// build from RexTmpVariable to RexGraphVariable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+ " values=[[{operands=[$f0], aggFunction=COUNT, alias='x', distinct=true}]])\n" | ||
+ " GraphLogicalProject($f0=[+(DEFAULT.age, 1)], isAppend=[true])\n" | ||
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}]," | ||
+ " alias=[~DEFAULT], opt=[VERTEX])", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unify the ~DEFAULT print
|
||
// group by HEAD.name, count(HEAD.age+1, 'x') -> project({HEAD.age+1 as '$f0'}, isAppend = true) | ||
// + aggregate(keys={HEAD.name}, calls=[count($f0) as 'x']) | ||
@Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refine the comments and add another test case for when agg key is an expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What do these changes do?
GraphBuilder
.GraphSchemaTypeList
, which contains schema meta for a vertex or an edge in fuzzy conditions.Related issue number
Fixes