Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11183 )
Change subject: IMPALA-7417: Speed up sub-second unix time->TimestampValue conversions ...................................................................... Patch Set 9: -Code-Review (1 comment) http://gerrit.cloudera.org:8080/#/c/11183/8/be/src/runtime/timestamp-value.inline.h File be/src/runtime/timestamp-value.inline.h: http://gerrit.cloudera.org:8080/#/c/11183/8/be/src/runtime/timestamp-value.inline.h@67 PS8, Line 67: const Timezone& local_tz) { The test failure in patch set 8 was caused by hitting this DCHECK for very small (<<1 nanosec) negative unix times. floor() truncated these values to -1, but unix_time - unix_time_whole was 1.0 (instead of 1-very_small_fractional _number) due the to the decreased preciison at 1.0 compared to 0.0. This would also be a problem when adding the sub-second value to time_, because adding a whole sec can step to a different day/timezone rule. The solution was to use rounding to get seconds, calculate nanoseconds from unix_time-unix_time_whole without any expectations, and give the results to FromUnixTimeNanos() , which can handle negative or >=1 sec values. This is probably a bit sub-optimal, but I do not want to put too much effort to this function. A benchmark was added that shows that this function is still faster than it used to be. -- To view, visit http://gerrit.cloudera.org:8080/11183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I572b5876b979ddae58165bd40d5b008ce9d7a4aa Gerrit-Change-Number: 11183 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Wed, 22 Aug 2018 11:12:14 +0000 Gerrit-HasComments: Yes