Dan Hecht has posted comments on this change.

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


Patch Set 1:

(2 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 574: *
> btw, yes, correct either mult==1 or div==1
Not sure what you mean by storing in thread local storage given that scale 
isn't a per-thread constant.


Line 599:       TimestampValue tv(whole_pt, fract_pt);
> Well, we don't support years larger than 10,000, so the whole part can't be
Okay, if the timestamp code already handles this could you please make sure we 
have test cases for the various edge cases, and also indicate which cases, if 
any, change behavior from the old to the new code?


-- 
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 <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Taras Bobrovytsky <[email protected]>
Gerrit-HasComments: Yes

Reply via email to