Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/13648 )
Change subject: IMPALA-7369: part 2: Add INTERVAL expr support and built-in functions for DATE ...................................................................... Patch Set 1: (7 comments) Thanks for taking a look. http://gerrit.cloudera.org:8080/#/c/13648/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13648/1//COMMIT_MSG@105 PS1, Line 105: 1 > This should be 0 according to the link. Done http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/exprs/expr-test.cc@7777 PS1, Line 7777: // 2019-01-01 is Tuesday, it belongs to the first wek of the year. > typo: week Done http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/exprs/expr-test.cc@7778 PS1, Line 7778: TestValue("weekofyear(date '2018-12-31')", TYPE_INT, 1); > The comment for WeekOfYear() states that it can return values 1-53. Can you Done http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc File be/src/runtime/date-test.cc: http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc@845 PS1, Line 845: // 2019-12-30 (Monday) and 2019-12-31 (Tuesday) belong to year 2020. > Can you add a test with week number 53? See expr-test.cc for details. Done http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc@910 PS1, Line 910: EXPECT_TRUE(DateValue(2016, 2, 29).MonthsBetween( : DateValue(2016, 1, 29), &months_between)); : EXPECT_EQ(1, months_between); > optional: a helper function like TestMonthsBetween(DateValue dt1, DateValue Done http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc@936 PS1, Line 936: monrhs_between > typo: months_between Done http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-value.h File be/src/runtime/date-value.h: http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-value.h@121 PS1, Line 121: AddMonths > I would prefer to move this to .cc / inline.h Done -- To view, visit http://gerrit.cloudera.org:8080/13648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If404bffdaf055c769e79ffa8f193bac415cfdd1a Gerrit-Change-Number: 13648 Gerrit-PatchSet: 1 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-Comment-Date: Mon, 17 Jun 2019 14:41:13 +0000 Gerrit-HasComments: Yes
