Skip to content

Fix regr functions result to be null when the input data is null#22112

Merged
tdcmeehan merged 1 commit intoprestodb:masterfrom
8dukongjian:main
Mar 15, 2024
Merged

Fix regr functions result to be null when the input data is null#22112
tdcmeehan merged 1 commit intoprestodb:masterfrom
8dukongjian:main

Conversation

@8dukongjian
Copy link
Contributor

@8dukongjian 8dukongjian commented Mar 7, 2024

Description

The result of the regr_count function should be null when the input data is null, not 0. The same problem exists with regr_avgx, regr_avgy, regr_syy, regr_sxx and regr_sxy.

presto> SELECT regr_count(c0, c1)
     -> FROM (
     ->   VALUES 
     ->     (null, null),
     ->     (null, null)
     -> ) AS tmp (c0, c1);
 _col0 
-------
   0.0 
(1 row)

Query 20240307_122037_00003_g2jsb, FINISHED, 1 node

fix #22110

Release Notes

== RELEASE NOTE ==

General Changes
* Fix the regr_count, regr_avgx, regr_avgy, regr_syy, regr_sxx, and regr_sxy functions result to be null when the input data is null, not 0. 

@8dukongjian 8dukongjian requested a review from a team as a code owner March 7, 2024 13:00
@8dukongjian 8dukongjian requested a review from presto-oss March 7, 2024 13:00
@8dukongjian
Copy link
Contributor Author

@tdcmeehan Would you help review this PR?

@steveburnett
Copy link
Contributor

Suggest adding a release note entry for this PR following the release notes guidelines. Perhaps something like this accurately describes the work?

== RELEASE NOTES ==

General Changes
* Fix the regr_count, regr_avgx, regr_avgy, regr_syy, regr_sxx, and regr_sxy functions result to be null when the input data is null, not 0. 

@8dukongjian
Copy link
Contributor Author

Suggest adding a release note entry for this PR following the release notes guidelines. Perhaps something like this accurately describes the work?

== RELEASE NOTES ==

General Changes
* Fix the regr_count, regr_avgx, regr_avgy, regr_syy, regr_sxx, and regr_sxy functions result to be null when the input data is null, not 0. 

Thanks, done.

@tdcmeehan
Copy link
Contributor

Please update the commit message to indicate the bugs we are fixing. Please ensure it follows our guidelines in CONTRIBUTING.md

double result = getRegressionSxy(state);
if (Double.isFinite(result)) {
double count = getRegressionCount(state);
if (Double.isFinite(result) && Double.isFinite(count) && count > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the count is not finite, should it throw?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the count is not finite, the result should be null, which is consistent with the behavior of the regr_count function.

@tdcmeehan tdcmeehan self-assigned this Mar 7, 2024
@8dukongjian 8dukongjian changed the title fix bug in regr functions Fix regr functions result to be null when the input data is null Mar 8, 2024
@8dukongjian
Copy link
Contributor Author

@tdcmeehan Could you help me review this, thank you.

@tdcmeehan
Copy link
Contributor

@8dukongjian please rebase to remove the merge commit. Then we can merge.

@8dukongjian
Copy link
Contributor Author

@8dukongjian please rebase to remove the merge commit. Then we can merge.

Done, Thanks!

@tdcmeehan tdcmeehan merged commit 3380324 into prestodb:master Mar 15, 2024
@wanglinsong wanglinsong mentioned this pull request May 1, 2024
48 tasks
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.

Regr_count function has incorrect results when the input data is null.

3 participants