Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/10486 )
Change subject: IMPALA-7060: Restrict Impala to only support timezones that work in Hive ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/10486/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10486/1//COMMIT_MSG@29 PS1, Line 29: I think you should add "Cherry-picks: not for 2.x." line. http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h File be/src/exprs/timezone_db.h: http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.h@55 PS1, Line 55: map Maybe use unordered_map instead ? Probably wouldn't make much of a difference though. http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc File be/src/exprs/timezone_db.cc: http://gerrit.cloudera.org:8080/#/c/10486/1/be/src/exprs/timezone_db.cc@68 PS1, Line 68: {"AEST", "Australia/Sydney"}, : {"CDT", "America/Chicago"}, : {"CEST", "CET"}, : {"EDT", "EST5EDT"}, : {"ICT", "Asia/Ho_Chi_Minh"}, : {"KST", "Asia/Seoul"}, : {"MDT", "MST7MDT"}, : {"PHT", "Asia/Manila"}, : {"PDT", "America/Los_Angeles"} Are these time-zone abbreviations accepted by Hive? If not, I think we should remove them. http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv File testdata/data/timezoneverification.csv: http://gerrit.cloudera.org:8080/#/c/10486/1/testdata/data/timezoneverification.csv@716 PS1, Line 716: JST Please make sure hat all the Java-supported time-zone abbreviations are tested here. -- To view, visit http://gerrit.cloudera.org:8080/10486 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90859398081bae4976af31b09b3121c198b6adac Gerrit-Change-Number: 10486 Gerrit-PatchSet: 1 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Comment-Date: Wed, 23 May 2018 17:53:08 +0000 Gerrit-HasComments: Yes
