Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3504: current timestamp in UTC, i.e. utc_timestamp() ......................................................................
Patch Set 1: (6 comments) You need to add some tests for this (in be/src/exprs/expr-test.cc) http://gerrit.cloudera.org:8080/#/c/3137/1/be/src/exprs/timestamp-functions-ir.cc File be/src/exprs/timestamp-functions-ir.cc: Line 47: using boost::posix_time::second_clock; Since all of these are only used once in this file, it doesn't really improve readability to use 'using' like this. You're probably better off just fully qualifying them where they're used. Line 823: StringVal TimestampFunctions::UtcTimestamp(FunctionContext* context){ space before '{' Line 824: ptime todayUtc(day_clock::universal_day(), today_utc What's the advantage here of combining day_clock::universal_day() with second_clock::universal_time().time_of_day()? Isn't this the same as just using second_clock::universal_time()? Line 826: std::string result = to_simple_string(todayUtc); It seems inefficient to convert the time into a string in the wrong format and then do the find and replace below. Is there a boost function that allows you to convert it directly to the right format? (eg. I see to_iso_extended_string in the docs) Line 830: if ( month_name == "Jan" ) { extra spaces inside the parentheses, here and below. Line 860: extraneous newlines -- To view, visit http://gerrit.cloudera.org:8080/3137 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9bfdb916aeb3a572c66d9dd786e3ea8342030ff0 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Youwei Wang <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
