Attila Jeges 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 6: (8 comments) http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/benchmarks/date-benchmark.cc File be/src/benchmarks/date-benchmark.cc: http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/benchmarks/date-benchmark.cc@89 PS6, Line 89: vector<DateValue> date; > nit: missing '_' at the end here and for the following member variables. Done http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/expr-test.cc@7626 PS6, Line 7626: "extract(date '2006-05-12', 'minute')" > should be 'expr'. Done. Not sure, how I messed these up :) http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/expr-test.cc@7632 PS6, Line 7632: "extract(date '2006-05-12', 'minute')" > should be 'expr'. Done http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/expr-test.cc@7661 PS6, Line 7661: "date_part('minute', date '2006-05-12')" > should be 'expr' Done http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/expr-test.cc@7667 PS6, Line 7667: "date_part('minute', date '2006-05-12')" > should be expr. Done http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/udf-builtins.h File be/src/exprs/udf-builtins.h: http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/udf-builtins.h@68 PS6, Line 68: /// WW : Same day of the week as the first day of the year : /// W : Same day of the week as the first day of the month > I'm not sure I understand these. Maybe you could add a '*' at end and expla Reworded the comment, hope it makes more sense now. http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/udf-builtins.h@125 PS6, Line 125: /// HOUR: The hour field (0–23). : /// MINUTE: The minutes field (0–59). : /// SECOND: The seconds field (0–59). : /// MILLISECONDS: The milliseconds fraction in the seconds. : /// MICROSECONDS: The microseconds fraction in the seconds. > very nit: maybe too much wording about stuff that it doesn't accept. Simple Done http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/udf-builtins.cc File be/src/exprs/udf-builtins.cc: http://gerrit.cloudera.org:8080/#/c/13363/6/be/src/exprs/udf-builtins.cc@661 PS6, Line 661: typename Val, : typename Value, > UdfType, InternalType? Done -- 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: 6 Gerrit-Owner: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 31 May 2019 13:47:24 +0000 Gerrit-HasComments: Yes
