Attila Jeges 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 14: Code-Review+1 (6 comments) Thanks, LGTM. Couple cosmetic things: http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/benchmarks/convert-timestamp-benchmark.cc File be/src/benchmarks/convert-timestamp-benchmark.cc: http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/benchmarks/convert-timestamp-benchmark.cc@581 PS14, Line 581: TimestampValue sec_split_utc_from_unix_time_micros( : const int64_t& unix_time_micros) { nit: one line. Here and below, where applicable. http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/benchmarks/convert-timestamp-benchmark.cc@875 PS14, Line 875: // Note that the test data contains only whole seconds. The comment is out of date. http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/benchmarks/convert-timestamp-benchmark.cc@902 PS14, Line 902: // Note that the test data contains only whole seconds. The comment is out of date. http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/benchmarks/convert-timestamp-benchmark.cc@929 PS14, Line 929: // Note that the test data contains only whole seconds. The comment is out of date. http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/benchmarks/convert-timestamp-benchmark.cc@936 PS14, Line 936: nit: extra space http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/runtime/timestamp-test.cc File be/src/runtime/timestamp-test.cc: http://gerrit.cloudera.org:8080/#/c/11183/14/be/src/runtime/timestamp-test.cc@280 PS14, Line 280: offset_millis Probably this should be called 'offset_seconds' or just 'offsets' -- 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: 14 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Mon, 03 Sep 2018 13:04:18 +0000 Gerrit-HasComments: Yes
