Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13363 )
Change subject: IMPALA-7369: part 1: Implement TRUNC, DATE_TRUNC, EXTRACT, DATE_PART functions for DATE ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/13363/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/13363/1/be/src/exprs/expr-test.cc@7431 PS1, Line 7431: . nit: inconsistent . at end of similar comments. http://gerrit.cloudera.org:8080/#/c/13363/1/be/src/exprs/expr-test.cc@7459 PS1, Line 7459: // Same day of the weekas the first day of the month type: week as http://gerrit.cloudera.org:8080/#/c/13363/1/be/src/exprs/expr-test.cc@7529 PS1, Line 7529: DateValue::Parse nit:A helper function like DateValue ParseDate(const char*) could make these tests less noisy and fit in one line. http://gerrit.cloudera.org:8080/#/c/13363/2/be/src/exprs/udf-builtins.cc File be/src/exprs/udf-builtins.cc: http://gerrit.cloudera.org:8080/#/c/13363/2/be/src/exprs/udf-builtins.cc@a18 PS2, Line 18: : The comments states that these functions were intentionally not cross compiled. See https://gerrit.cloudera.org/#/c/7081/ for the details. I would prefer to keep these functions at their original place unless we can verify that codegen speeds up these functions significantly. My guess is that the cost of splitting 'days since epoch' to year/month/day (which cannot be avoided by codegen) is slow enough to make the extra function call ( + switch ? ) insignificant. -- To view, visit http://gerrit.cloudera.org:8080/13363 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I843358a45eb5faa2c134994600546fc1d0a797c8 Gerrit-Change-Number: 13363 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 20 May 2019 14:28:16 +0000 Gerrit-HasComments: Yes
