impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5137: Support Kudu UNIXTIME MICROS as Impala TIMESTAMP
Date Tue, 09 May 2017 00:55:17 GMT
Marcel Kornacker has posted comments on this change.

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

Patch Set 7: Code-Review+2

File be/src/exec/kudu-util.h:

Line 89:       if (COPY_STRINGS) {
it doesn't feel like this particular if should be very performance-relevant. let's make it
a standard function again, with an extra bool param.
File be/src/runtime/timestamp-value.h:

Line 1: // Licensed to the Apache Software Foundation (ASF) under one
this file is included in bunch of .h files. now that you pulled out a bunch of function bodies,
those includes may not be necessary anymore. (i see one in util/promise.h, for instance).
would be nice to clean that up as well.

doesn't have to be as part of this change, but it's good to do some housecleaning as you go.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae6ccfffb79118a9036fb2227dba3a55356c896d
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-HasComments: Yes

View raw message