Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3504: current timestamp in UTC, i.e. utc_timestamp()
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3137/3/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:

Line 818: UtcTimestamp
This JIRA isn't well defined.

I still think this isn't the right thing to do for the UDF. Notice that the 
now() builtin for the local timestamp (TimestampFunctions::Now()) uses a 
query-wide global that comes from RuntimeState::now() (and ultimately was set 
by the coordinator in ImpalaServer::PrepareQueryContext(). That allows us to 
have a consistent value for now() whenever the UDF happens to be executed. 
Think about a query like 

 select count(*), utc_timestamp() from functional.alltypesagg group by 
tinyint_col;

The function gets evaluated many times. Consider more complicated queries. The 
function could be evaluated on completely different nodes (clock skew?). The 
results won't make sense.

I suspect we want to capture the UTC time on the coordinator and setting it in 
the TQueryCtx (see impala-server.cc:840) like we do for local time. Then if 
different hosts have skew and/or inconsistent settings we'd get the same 
(consistent) value everywhere. I mentioned this on the JIRA, let's have a 
discussion about the desired behavior there.


Line 820:   string result = 
to_iso_extended_string(second_clock::universal_time());
        :   result[10] = ' '; //erase 'T' character in the extended ISO 
representation
        :   StringVal date = StringVal::CopyFrom(context,
        :       reinterpret_cast<const uint8_t*>(result.c_str()), 
result.size());
        :   BigIntVal biv = TimestampFunctions::UnixFromString(context, date);
I don't think we'll want to do this. See my previous comment. I think a better 
solution will be to have a function like TimestampValue::UtcToLocal() but 
reversed, e.g. TimestampValue::LocalToUtc(), and then


-- 
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: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Youwei Wang <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Youwei Wang <[email protected]>
Gerrit-HasComments: Yes

Reply via email to