impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5137: Support Kudu UNIXTIME MICROS as Impala TIMESTAMP
Date Fri, 21 Apr 2017 00:42:49 GMT
Matthew Jacobs has posted comments on this change.

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

Patch Set 2:


Thanks for the input Lars, sorry for the delay. I've been waiting to do the read path as well.
I have a patch I'll post soon.
File be/src/exec/

Line 302:           KUDU_RETURN_IF_ERROR(write->mutable_row()->SetUnixTimeMicros(col,
> This DCHECK effectively tests tv->HasDateAndTime(). Is there any way this c
HasDateAndTime checks that both the boost date or boost time are not 'special' values (ie.
+/- inf, not_a_date_time). If these are 'special' the slot *should be* null. AFAIK there is
no way to represent +/- inf or 'not_a_date_time' as an Impala TIMESTAMP -- they'd be NULL.

timestamp-value.h doesn't really explain, this is all based on me reading the code, so if
you or others think otherwise please let me know :)

That said, I guess this should be defensive and also RETURN_IF_FALSE for release builds, so
I'll continue to DCHECK but will also RETURN_IF_FALSE for avoiding unexpected behavior in
release builds.
File fe/src/main/java/org/apache/impala/planner/

Line 125:       Type colType = desc.getColumn().getType();
> Nit: Now that I look at this you could also inline it. But I'm don't feel s
this goes away anyway :)
File fe/src/main/java/org/apache/impala/util/

Line 186:       case UNIXTIME_MICROS:
> Where did this go?
I'm going to do all the FE-related changes in a future patch. (And this is going to be a bit
more tricky than it looks because the literal needs to be converted to a unixtime micros from
a Timestamp.)

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae6ccfffb79118a9036fb2227dba3a55356c896d
Gerrit-PatchSet: 2
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: Matthew Jacobs <>
Gerrit-HasComments: Yes

View raw message