-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Feature] (Part2) Support _state/_union/_merge aggregate funciton combinator #50425
[Feature] (Part2) Support _state/_union/_merge aggregate funciton combinator #50425
Conversation
c85e66c
to
e00b08f
Compare
3569d80
to
deb408d
Compare
008c23c
to
21f5e67
Compare
@mER�gify rebase |
@mergify rebase |
21f5e67
to
11ec2df
Compare
✅ Branch has been successfully rebased |
.add(COUNT) // Functions with pseudo types are not supported. | ||
.add(COUNT_IF) | ||
.add(FLAT_JSON_META) // Internal functions are not supported. | ||
.add(HLL_RAW) |
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.
COUNT, COUNT_IF is used frequently, so we need to support it.
NDV/APPROX_COUNT_DISTINCT(c) = HLL_CARDINALITY(HLL_RAW(c));
HLL_CARDINALITY(HLL_UNION(HLL_HASH(c))) = HLL_CARDINALITY(HLL_RAW(cast(c as STRING)))
so, in fact, we only need to implement two hll agg functions, hll_raw和hll_union, other functions derived agg functions need not be re-implemented, just use other functions to convert input/output parameters.
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.
count/array_agg
have been supportedhll_raw
is not supported because it useshll
as input which has been supported inhll_union
in aggregate table.- you are right, it's only true that we only need to implement some agg functions' state(eg: array_agg) to recompute other functions state which may be like implementions in the streaming system. But that's a huge work to recompute agg functions by the limited agg function state and we can discuss this later.
e4c497b
to
5087127
Compare
5087127
to
d5a73b6
Compare
@mergify rebase |
Signed-off-by: shuming.li <[email protected]>
✅ Branch has been successfully rebased |
d5a73b6
to
d9c4715
Compare
Quality Gate failedFailed conditions |
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]❌ fail : 65 / 222 (29.28%) file detail
|
[FE Incremental Coverage Report]✅ pass : 339 / 379 (89.45%) file detail
|
…binator (StarRocks#50425) Signed-off-by: shuming.li <[email protected]>
…binator (StarRocks#50425) Signed-off-by: shuming.li <[email protected]> Signed-off-by: zhiminr.ren <[email protected]>
Why I'm doing:
_state
combinator_union
combinator_merge
combinatorWhat I'm doing:
Fixes #49000
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: