Skip to content

[Enhancement](doris-future) Support "REGR_" aggregation functions (PART II) - #41240

Merged
zhangstar333 merged 19 commits into
apache:masterfrom
Yoruet:master
Oct 8, 2024
Merged

[Enhancement](doris-future) Support "REGR_" aggregation functions (PART II)#41240
zhangstar333 merged 19 commits into
apache:masterfrom
Yoruet:master

Conversation

@Yoruet

@YoruetYoruet commented Sep 24, 2024

Copy link
Copy Markdown
Contributor

Proposed changes

Issue Number: close#38975

mysql>select*from test;
+------+------+------+
| id | x | y |
+------+------+------+
| 1 | 18 | 13 |
| 3 | 12 | 2 |
| 5 | 10 | 20 |
| 2 | 14 | 27 |
| 4 | 5 | 6 |
+------+------+------+5 rows inset (0.07 sec)
mysql>select regr_slope(y,x) , regr_intercept(y,x) from test;
+--------------------+----------------------+
| regr_slope(y, x) | regr_intercept(y, x) |
+--------------------+----------------------+
| 0.6853448275862069 | 5.512931034482759 |
+--------------------+----------------------+1 row inset (0.15 sec)

@doris-robot

Copy link
Copy Markdown

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@YoruetYoruet changed the title add regr_intercept and regr_slope aggregate functions[Enhancement](doris-future) Support "REGR_" aggregation functions (PART II)Sep 24, 2024
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_slope.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_slope.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
@RoanHeNaN

Copy link
Copy Markdown
Contributor

@Yoruet Null property of regr_slope and regr_intercept should be AlwaysNullable on FE, so need to do some modification on be.

@RoanHeNaN

Copy link
Copy Markdown
Contributor

@Yoruet You can take this pr #40945 as a reference.

Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.cpp Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
@RoanHeNaN

Copy link
Copy Markdown
Contributor

@Yoruet I add more comments with detail information.

Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.cpp Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_intercept.h Outdated
@RoanHeNaN

RoanHeNaN commented Sep 27, 2024

Copy link
Copy Markdown
Contributor

Yoruet#1

@Yoruet I did some fix on regr_intercept on your branch, you can merge the pull request and do modification on other function. Test case should be taked.

Reference https://dbfiddle.uk/MmKrIU9r

@RoanHeNaNRoanHeNaN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Null value is not processed correctly.

@Yoruet

Copy link
Copy Markdown
ContributorAuthor

Null value is not processed correctly.

Can you provide corresponding test samples to me

@RoanHeNaN

Copy link
Copy Markdown
Contributor

Null value is not processed correctly.

Can you provide corresponding test samples to me

@Yoruet You can try test case added in this pr Yoruet#1 on regr_slope

@Yoruet

Yoruet commented Sep 27, 2024

Copy link
Copy Markdown
ContributorAuthor

Null value is not processed correctly.

Can you provide corresponding test samples to me

@Yoruet You can try test case added in this pr Yoruet#1 on regr_slope

Null value is not processed correctly.

Can you provide corresponding test samples to me

@Yoruet You can try test case added in this pr Yoruet#1 on regr_slope

@zhiqiang-hhhh I compared the data in the test_regr_intercept.out file with that in https://dbfiddle.uk/MmKrIU9r, and it seems that there is nothing wrong. Can you provide more detailed information?

@RoanHeNaN

Copy link
Copy Markdown
Contributor

Null value is not processed correctly.

Can you provide corresponding test samples to me

@Yoruet You can try test case added in this pr Yoruet#1 on regr_slope

Null value is not processed correctly.

Can you provide corresponding test samples to me

@Yoruet You can try test case added in this pr Yoruet#1 on regr_slope

@zhiqiang-hhhh I compared the data in the test_regr_intercept.out file with that in https://dbfiddle.uk/MmKrIU9r, and it seems that there is nothing wrong. Can you provide more detailed information?

The reason test_regr_intercept is correct is that I re-write the implementation of regr_itercept in that PR (I created that pull request on your branch to give you an example on how to fix regr_itercept).

You can compare the code I submitted with your implementation of regr_itercept, and do fix on regr_slope in a same way.

@Yoruet

Yoruet commented Sep 29, 2024

Copy link
Copy Markdown
ContributorAuthor

We need todo a refactor on all REGR_ like agg functions, to make code simple. These agg functions have same structure. eg. #39187

Does it include only regr_slope and regr_intercept or does it include other regr_ functions such as regr_sxx

@RoanHeNaN

Copy link
Copy Markdown
Contributor

We need todo a refactor on all REGR_ like agg functions, to make code simple. These agg functions have same structure. eg. #39187

Does it include only regr_slope and regr_intercept or does it include other regr_ functions such as regr_sxx

This is just a comment to remind us to do a refactor in the future, does not means we need to do refactor in this pr.

@HappenLee

Copy link
Copy Markdown
Contributor

@Yoruet please add doc in https://github.com/apache/doris-website thank you very mush

@Yoruet

Copy link
Copy Markdown
ContributorAuthor

@Yoruet please add doc in https://github.com/apache/doris-website thank you very mush

I have already pr the doc.plz check it

Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_slope.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_union.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_union.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_union.h Outdated
Comment threadbe/src/vec/aggregate_functions/aggregate_function_regr_union.h Outdated
@Yoruet
Yoruet requested a review from HappenLeeOctober 2, 2024 11:34

@HappenLeeHappenLee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Oct 3, 2024
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@HappenLee

Copy link
Copy Markdown
Contributor

run buildall

@doris-robot

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 37.30% (9632/25825)
Line Coverage: 28.68% (79854/278453)
Region Coverage: 28.11% (41291/146877)
Branch Coverage: 24.74% (21039/85034)
Coverage Report: http://coverage.selectdb-in.cc/coverage/68d8d740c68642db67051a62d6b3b8c18319ac4e_68d8d740c68642db67051a62d6b3b8c18319ac4e/report/index.html

@zhangstar333
zhangstar333 merged commit 0e5fd2f into apache:masterOct 8, 2024
eldenmoon pushed a commit to eldenmoon/incubator-doris that referenced this pull request Oct 10, 2024
…RT II) (apache#41240)
## Proposed changes
Issue Number: closeapache#38975
<!--Describe your changes.-->
```sql
mysql> select * from test;
+------+------+------+
| id | x | y |
+------+------+------+
| 1 | 18 | 13 |
| 3 | 12 | 2 |
| 5 | 10 | 20 |
| 2 | 14 | 27 |
| 4 | 5 | 6 |
+------+------+------+
5 rows in set (0.07 sec)
mysql> select regr_slope(y,x) , regr_intercept(y,x) from test;
+--------------------+----------------------+
| regr_slope(y, x) | regr_intercept(y, x) |
+--------------------+----------------------+
| 0.6853448275862069 | 5.512931034482759 |
+--------------------+----------------------+
1 row in set (0.15 sec)
```
---------
Co-authored-by: zhiqiang-hhhh <seuhezhiqiang@163.com>
cjj2010 pushed a commit to cjj2010/doris that referenced this pull request Oct 12, 2024
…RT II) (apache#41240)
## Proposed changes
Issue Number: closeapache#38975
<!--Describe your changes.-->
```sql
mysql> select * from test;
+------+------+------+
| id | x | y |
+------+------+------+
| 1 | 18 | 13 |
| 3 | 12 | 2 |
| 5 | 10 | 20 |
| 2 | 14 | 27 |
| 4 | 5 | 6 |
+------+------+------+
5 rows in set (0.07 sec)
mysql> select regr_slope(y,x) , regr_intercept(y,x) from test;
+--------------------+----------------------+
| regr_slope(y, x) | regr_intercept(y, x) |
+--------------------+----------------------+
| 0.6853448275862069 | 5.512931034482759 |
+--------------------+----------------------+
1 row in set (0.15 sec)
```
---------
Co-authored-by: zhiqiang-hhhh <seuhezhiqiang@163.com>
amorynan pushed a commit to amorynan/doris that referenced this pull request Oct 12, 2024
…RT II) (apache#41240)
## Proposed changes
Issue Number: closeapache#38975
<!--Describe your changes.-->
```sql
mysql> select * from test;
+------+------+------+
| id | x | y |
+------+------+------+
| 1 | 18 | 13 |
| 3 | 12 | 2 |
| 5 | 10 | 20 |
| 2 | 14 | 27 |
| 4 | 5 | 6 |
+------+------+------+
5 rows in set (0.07 sec)
mysql> select regr_slope(y,x) , regr_intercept(y,x) from test;
+--------------------+----------------------+
| regr_slope(y, x) | regr_intercept(y, x) |
+--------------------+----------------------+
| 0.6853448275862069 | 5.512931034482759 |
+--------------------+----------------------+
1 row in set (0.15 sec)
```
---------
Co-authored-by: zhiqiang-hhhh <seuhezhiqiang@163.com>
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Apr 24, 2026
…RT II) (apache#41240)
## Proposed changes
Issue Number: closeapache#38975
<!--Describe your changes.-->
```sql
mysql> select * from test;
+------+------+------+
| id | x | y |
+------+------+------+
| 1 | 18 | 13 |
| 3 | 12 | 2 |
| 5 | 10 | 20 |
| 2 | 14 | 27 |
| 4 | 5 | 6 |
+------+------+------+
5 rows in set (0.07 sec)
mysql> select regr_slope(y,x) , regr_intercept(y,x) from test;
+--------------------+----------------------+
| regr_slope(y, x) | regr_intercept(y, x) |
+--------------------+----------------------+
| 0.6853448275862069 | 5.512931034482759 |
+--------------------+----------------------+
1 row in set (0.15 sec)
```
---------
Co-authored-by: zhiqiang-hhhh <seuhezhiqiang@163.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement](doris-future) Support "REGR_" aggregation functions (PART II)

5 participants

@Yoruet@doris-robot@RoanHeNaN@HappenLee@zhangstar333