impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Taras Bobrovytsky (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-2.6.0_5.8.0) IMPALA-3163: Fix Decimal to Timestamp casting
Date Mon, 23 May 2016 19:31:34 GMT
Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-3163: Fix Decimal to Timestamp casting
......................................................................


Patch Set 1: -Code-Review

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3154/1/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

Line 565: TIMESTAMP_PRECISION
> TIMESTAMP_SCALE (to be consistent with decimal terminology, and reword the 
Done


Line 574: *
> oh, i guess what's happening here is that we always have either mult==1 or 
Yeah that's what I was thinking initially, but there way this way it's a few percent faster
according to my benchmarks (because there is no branch). Initially the way I implemented this
was to precompute and store the mult and div coefficients in thread local storage. Strangely
that turned out to be slower than this implementation (probably because reading from that
storage is slow). I think it makes sense to implement it with the if statement because it's
more readable and easier to understand.


Line 599:       TimestampValue tv(whole_pt, fract_pt);
> what is the effective (precision, scale) for TIMESTAMP? aren't there some d
Well, we don't support years larger than 10,000, so the whole part can't be bigger than about
250 billion. So when we convert an integer or double to timestamp that's larger than that,
it just becomes NULL. If the scale of the decimal is larger than 9 (nanosecond), I think it
makes sense to just reduce it to 9 (that's what we're doing now) without issuing any warnings,
I doubt anyone expects the precision be larger than that.


-- 
To view, visit http://gerrit.cloudera.org:8080/3154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabeea9f4ab4880b2f814408add63c77916e2dba9
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message