Dan Hecht has posted comments on this change. Change subject: IMPALA-4546: Fix Moscow timezone conversion after 2014 ......................................................................
Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/5969/6/be/src/exprs/timestamp-functions.cc File be/src/exprs/timestamp-functions.cc: Line 133: // NULL". > Yes, I checked, the ambiguity is checked in the constructor of local_date_t My suggestion was to move this code to be close to the constructor, since that's where the other validation is handled. i.e. at least move the if-stmt at line 138 up above this block so that the related code is grouped together. And then, if we were to pull it out, that subroutine would include this block along with some of the block at line 146. But you don't have to do that. -- 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
