impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3504: current timestamp in UTC, i.e. utc_timestamp()
Date Fri, 20 May 2016 22:29:40 GMT
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 <429222616@qq.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message