[email protected] has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19569 )

Change subject: IMPALA-11957: Implement Regression functions: regr_slope(), 
regr_intercept() and regr_r2()
......................................................................


Patch Set 10:

(6 comments)

> Patch Set 9:
>
> (7 comments)

http://gerrit.cloudera.org:8080/#/c/19569/7/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/19569/7/be/src/exprs/aggregate-functions-ir.cc@702
PS7, Line 702: ignored and rounded
> I'll try to reproduce the situation myself and see what Hive does.
Sure


http://gerrit.cloudera.org:8080/#/c/19569/9/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/19569/9/be/src/exprs/aggregate-functions-ir.cc@683
PS9, Line 683: =
> There's no matching opening parenthesis. I think the opening paren should c
Done


http://gerrit.cloudera.org:8080/#/c/19569/9/be/src/exprs/aggregate-functions-ir.cc@685
PS9, Line 685: // where y and x are the dependent and independent variables
> Fits on the previous line, after the if.
Done


http://gerrit.cloudera.org:8080/#/c/19569/9/be/src/exprs/aggregate-functions-ir.cc@692
PS9, Line 692:   // 'x_var'. This is to avoid confusion, because for regr_r2() 
the dependent variable is
             :   // the first parameter and the independent variable is the 
second parameter, but
             :   // CorrUpdate(), which we use to produce the intermediate 
values, has the o
> This could come after assigning 'dependent_var' and 'independent_var', befo
Done


http://gerrit.cloudera.org:8080/#/c/19569/9/be/src/exprs/aggregate-functions-ir.cc@712
PS9, Line 712: d_squared' can only be 0 if eit
> Shouldn't we also divide these by state->count? In that case we could inclu
No, because covariance will also get divided by state->count which will cancel 
the divisions of dependent_var and independent_var.


http://gerrit.cloudera.org:8080/#/c/19569/9/be/src/exprs/aggregate-functions-ir.cc@714
PS9, Line 714: ind
> Nit: is.
Done



--
To view, visit http://gerrit.cloudera.org:8080/19569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab6bd84ae3e0c02ec924c30183308123b951caa3
Gerrit-Change-Number: 19569
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Wed, 17 May 2023 20:16:19 +0000
Gerrit-HasComments: Yes

Reply via email to