impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3504: UDF for current timestamp in UTC
Date Wed, 21 Jun 2017 05:11:16 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3504: UDF for current timestamp in UTC

Patch Set 3:

File be/src/runtime/runtime-state.h:

Line 343:   /// Use pointer to avoid inclusion of timestampvalue.h and avoid clang issues.
Given the names are so different that it's not obvious (yet the names are okay since they
are consistent with the built-ins they implement), I think it would help to document that
these are the same point in time but that now is in local time and utc_timestamp_ is in UTC.
File be/src/runtime/timestamp-value.h:

Line 124:   }
not your change and you shouldn't fix it in this commit, but all remaining uses of LocalTime()
seem wrong. We should't be using TimestampValue as a general timer mechanism -- it should
just be used for the TIMESTAMP datatype (and additionally some of the uses don't make much
sense in local time).
File common/thrift/ImpalaInternalService.thrift:

Line 371:   // String containing a timestamp (in UTC) set at the query submission time.
document same point in time as 'now_string'

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I969fc805922f2bb9c8101e84f85ff2cc3b1b6729
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig <>
Gerrit-Reviewer: Bikramjeet Vig <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Jim Apple <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-HasComments: Yes

View raw message