Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/18413 )
Change subject: IMPALA-11205: Implement Statistical functions : CORR(), COVAR_SAMP() and COVAR_POP() ...................................................................... Patch Set 18: (8 comments) http://gerrit.cloudera.org:8080/#/c/18413/18/be/src/exprs/aggregate-functions-ir.cc File be/src/exprs/aggregate-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/18413/18/be/src/exprs/aggregate-functions-ir.cc@408 PS18, Line 408: if (isnan(val1) || isnan(val2)) return; Skipping NaN here seems questionable? Please add comments or handle appropriately. http://gerrit.cloudera.org:8080/#/c/18413/18/be/src/exprs/aggregate-functions-ir.cc@422 PS18, Line 422: if (nA == 0) { Does (nB == 0) need handling? http://gerrit.cloudera.org:8080/#/c/18413/18/be/src/exprs/aggregate-functions-ir.cc@543 PS18, Line 543: if (src1.is_null || isnan(src1.val) || src2.is_null || isnan(src2.val)) return; Skipping Nan? http://gerrit.cloudera.org:8080/#/c/18413/18/be/src/exprs/aggregate-functions-ir.cc@575 PS18, Line 575: if (isnan(val1) || isnan(val2)) return; Skipping NaN http://gerrit.cloudera.org:8080/#/c/18413/18/be/src/exprs/aggregate-functions-ir.cc@589 PS18, Line 589: if (nA == 0) { handle (nB == 0) ? http://gerrit.cloudera.org:8080/#/c/18413/18/testdata/workloads/functional-query/queries/QueryTest/aggregation.test File testdata/workloads/functional-query/queries/QueryTest/aggregation.test: http://gerrit.cloudera.org:8080/#/c/18413/18/testdata/workloads/functional-query/queries/QueryTest/aggregation.test@1526 PS18, Line 1526: select s_store_sk, corr(s_number_employees, s_floor_space) over (partition by s_city) from tpcds.store; This may be non-deterministic (flaky) without an order by on the window clause. http://gerrit.cloudera.org:8080/#/c/18413/18/testdata/workloads/functional-query/queries/QueryTest/aggregation.test@1544 PS18, Line 1544: select id, double_col, corr(double_col, int_col) over (partition by month) from functional.alltypes This may be non-deterministic (flaky) without an order by on the window clause. http://gerrit.cloudera.org:8080/#/c/18413/18/testdata/workloads/functional-query/queries/QueryTest/aggregation.test@1578 PS18, Line 1578: select s_store_sk, corr(s_number_employees, s_floor_space) over (partition by s_city order by s_city Check if order by city is strong ordering. -- 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: 18 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: Wed, 01 Jun 2022 18:38:53 +0000 Gerrit-HasComments: Yes
