[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/18413 )
Change subject: IMPALA-11205: Implement CORR() function ...................................................................... Patch Set 10: (3 comments) > Patch Set 10: > > (4 comments) > > I would prefer to see the covar_pop and covar_samp functions implemented in > the same patch as well. Since this jira was focused on corr(), should we change its description or create a new jira for covariance functions? http://gerrit.cloudera.org:8080/#/c/18413/10/be/src/exprs/aggregate-functions-ir.cc File be/src/exprs/aggregate-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/18413/10/be/src/exprs/aggregate-functions-ir.cc@296 PS10, Line 296: double prod; > These and any other computation variables may need to be precise types if t AFAIK, a similar approach is used in hive and other aggregate functions of impala, that's why we kept it double. Double supports precision types like Decimal. There's a test included as well just to check this behaviour. http://gerrit.cloudera.org:8080/#/c/18413/10/be/src/exprs/aggregate-functions-ir.cc@317 PS10, Line 317: state->sumx += x; > Handle overflow? AFAIK, Hive and other aggregate functions of impala like avg() aren't checking explicitly for overflow. Do you want any specific check or behavior in case of overflow, probably returning null or giving an error? http://gerrit.cloudera.org:8080/#/c/18413/10/be/src/exprs/aggregate-functions-ir.cc@389 PS10, Line 389: dst_state->sumx += src_state->sumx; > Handle overflow? AFAIK, Hive and other aggregate functions of impala like avg() aren't checking explicitly for overflow. Do you want any specific check or behavior in case of overflow, probably returning null or giving an error? -- To view, visit http://gerrit.cloudera.org:8080/18413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32ad627c953ba24d9cde2d5549bdd0d27a9c0d06 Gerrit-Change-Number: 18413 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Fri, 29 Apr 2022 07:03:57 +0000 Gerrit-HasComments: Yes
