Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-5137: Support Kudu UNIXTIME_MICROS as Impala TIMESTAMP
......................................................................


Patch Set 5:

(2 comments)

done with the comments.

http://gerrit.cloudera.org:8080/#/c/6526/5/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

Line 193:     tm temp_tm = boost::posix_time::to_tm(temp);
what about to_time_t, or does that require a newer version of boost?

if the latter, what about what's reported here: 
http://stackoverflow.com/questions/4461586/how-do-i-convert-boostposix-timeptime-to-time-t

time_t to_time_t(boost::posix_time::ptime t)
{
    using namespace boost::posix_time;
    ptime epoch(boost::gregorian::date(1970,1,1));
    time_duration::sec_type x = (t - epoch).total_seconds();

    // ... check overflow here ...

    return time_t(x);
}


Line 214:     *unix_time_micros = std::min(MAX_UNIXTIME_MICROS, 
*unix_time_micros);
should this return false when you go above the max?


-- 
To view, visit http://gerrit.cloudera.org:8080/6526
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae6ccfffb79118a9036fb2227dba3a55356c896d
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <m...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to