Dan Hecht has posted comments on this change. Change subject: IMPALA-4546: Fix Moscow timezone conversion after 2014 ......................................................................
Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/5969/6/be/src/exprs/timestamp-functions.cc File be/src/exprs/timestamp-functions.cc: Line 133: // NULL". > Alex and I came to this decision together. If we do this check inside FindT Okay, I guess what you're saying is that for the ts_val's in this range, the timezone is definitely MSK_PRE_2014, but you can't convert it to UTC since it's ambiguous. But, doesn't that happen for other timezone/timestamps as well? Oh, I guess not since the timezone is specified and usually differentiates between daylight savings and standard. Though in the tests for the 2011 cases where there are ambiguity, what cases are those and what code path does it take? PS6, Line 134: DCHECK DCHECK_LT PS6, Line 205: March 27, 2011 this doesn't seem to match the code for March 28-30th... what's the story with that? PS6, Line 205: NOTE: We currently : // do not handle Moscow time conversions for dates before January 19, 1992 : // correctly (Impala incorrectly thinks the Moscow timezone is UTC+3 with DST : // instead of UTC+2 with DST for those dates). is this a pre-existing bug? is there a jira? -- 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: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
