Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4546: Fix Moscow timezone conversion after 2014 ......................................................................
Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/5969/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 3588: // IMPALA-4209: Moscow time change in 2011. > Moscow could use a test on its own, rather than adding so many lines to Tim Done PS1, Line 3590: cast(from_utc_timestamp('2010-10-30 22:59:00', " : "'Europe/Moscow') as string) > This could be turned into a template of calling TestStringValue with "cast( Done Line 3596: TestIsNull("cast(to_utc_timestamp('2010-10-31 02:00:00', " > Each one of these lines could use a short comment like "spring forward into Done http://gerrit.cloudera.org:8080/#/c/5969/1/be/src/exprs/timestamp-functions.cc File be/src/exprs/timestamp-functions.cc: PS1, Line 194: January 19, 1992 > What about in 1991? Added comment. In general, Timestamp timezone conversions are broken. http://gerrit.cloudera.org:8080/#/c/5969/1/be/src/exprs/timezone_db.h File be/src/exprs/timezone_db.h: PS1, Line 41: "tz" > 'tz', because the parameter tz is a std::string and so "tz" is a valid valu Done Line 45: /// Moscow timezone UTC+3 with DST, for use before 27 March 2011. > Between 27 March 2011 and 26 October 2014, which timezone was Moscow in? Moscow was in TIMEZONE_MSK_PRE_2014 timezone between 2011 and 2014, which is UTC+4 PS1, Line 48: October 26 2014 > "26 October 2014" for parallelism with above comment. I changed both to Month, Day, Year. -- To view, visit http://gerrit.cloudera.org:8080/5969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id6e3f2c9f6ba29749a26bc1087e664637bc02528 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-HasComments: Yes
