[Enhancement](doris-future) Support REGR_SXX_SXY_SYY aggregation functions#39187
[Enhancement](doris-future) Support REGR_SXX_SXY_SYY aggregation functions#39187wyxxxcat wants to merge 3 commits intoapache:masterfrom
Conversation
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
969b94a to
3bc2fcd
Compare
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/RegrSyy.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/RegrSyy.java
Outdated
Show resolved
Hide resolved
68a44ac to
a441b02
Compare
|
run buildall |
TPC-H: Total hot run time: 39846 ms |
|
测试注意这些点都得覆盖:
|
a441b02 to
bf50101
Compare
regression-test/suites/nereids_function_p0/agg_function/test_regr_sxy.groovy
Outdated
Show resolved
Hide resolved
bf50101 to
1c72316
Compare
|
run buildall |
07efe15 to
558df01
Compare
558df01 to
c6b1da4
Compare
|
run buildall |
morrySnow
left a comment
There was a problem hiding this comment.
add doc in https://github.com/apache/doris-website
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/RegrSxx.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/RegrSxx.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/RegrSxy.java
Outdated
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/RegrSyy.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/apache/doris/nereids/trees/expressions/visitor/AggregateFunctionVisitor.java
Show resolved
Hide resolved
fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinAggregateFunctions.java
Outdated
Show resolved
Hide resolved
TPC-H: Total hot run time: 37911 ms |
Done |
e2eaf30 to
ea50916
Compare
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 40569 ms |
TPC-DS: Total hot run time: 191577 ms |
ClickBench: Total hot run time: 32.86 s |
ccc16f4 to
f5a0bf2
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@zhiqiang-hhhh plz review |
| } | ||
|
|
||
| Float64 get_regr_sxx_result() const { | ||
| Float64 result = sum_of_x_squared - (sum_x * sum_x) / count; |
There was a problem hiding this comment.
what if count is zero
zhiqiang-hhhh
left a comment
There was a problem hiding this comment.
We should use template to avoid redundant agg data field.
| Float64 sum_y {}; | ||
| Float64 sum_of_x_mul_y {}; | ||
| Float64 sum_of_x_squared {}; | ||
| Float64 sum_of_y_squared {}; |
There was a problem hiding this comment.
sum_of_y_squared is only needed for regr_syy, in other situations, this field is not necessary.
so we should use template to make our implementation more efficient.
f5a0bf2 to
ca7ff07
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
zhiqiang-hhhh
left a comment
There was a problem hiding this comment.
- Need refactor to simplify code.
- Need more test.
| Float64 get_regr_sxx_result() const { | ||
| // count == 0 | ||
| // The result of a query for an empty table is a null value | ||
| Float64 result = sum_of_x_squared - (sum_x * sum_x / count); |
There was a problem hiding this comment.
if (count == 0) {
return Nan;
}
| Float64 get_regr_sxy_result() const { | ||
| // count == 0 | ||
| // The result of a query for an empty table is a null value | ||
| Float64 result = sum_of_x_mul_y - (sum_x * sum_y / count); |
There was a problem hiding this comment.
if (count == 0 ) {
return Nan;
}
| Float64 get_regr_syy_result() const { | ||
| // count == 0 | ||
| // The result of a query for an empty table is a null value | ||
| Float64 result = sum_of_y_squared - (sum_y * sum_y / count); |
There was a problem hiding this comment.
if count == 0 {
return Nan;
}
|
@wyxxxcat Could you please contact me on wechat? 839616693 |
|
We're closing this PR because it hasn't been updated in a while. |
…onRegrData (#59224) ### What problem does this PR solve? Issue Number: close #38977 Problem Summary: This PR migrates regr_sxx/syy/sxy onto the shared Moment(AggregateFunctionRegrData) introduced in #55940. The original implementation and tests were done in #39187 by @wyxxxcat. This PR builds on top of that work, refactoring it to reuse the same state and merge logic. --------- Co-authored-by: wyxxxcat <1520358997@qq.com>
Proposed changes
Issue Number: #38977
doc : apache/doris-website#1014