Dan Hecht has posted comments on this change.

Change subject: IMPALA-2020: Add rounding when casting from decimal to int
......................................................................


Patch Set 3:

(2 comments)

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

PS3, Line 297: round ? dv.ToIntVal<to_type>(scale) : 
to_type(dv.whole_part(scale));
perhaps pass 'round' to ToInt() and move the "else" block in there since it's 
basically the truncation case. that way we're more symmetric.

and then the to_type can stay here, and instead ToInt() can return the 
primitive (like whole_part() does), which would keep all the conversions 
between DecimalValue and udf::*IntVal types within this code, which I think 
makes more sense since this code is the UDF builtin code, whereas DecimalValue 
is the decimal slot representation code internal to impala.


http://gerrit.cloudera.org:8080/#/c/5951/3/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

Line 88:     return RESULT_T::null();
> That's a good idea.  Should I still be returning raw UDF types (in which ca
I would probably leave the UDF type (e.g. BigIntVal) out of here and just 
return the raw primitive, unless using the UDF type simplifies things for some 
reason.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2daf186b4770a022f9cb349d512067a1dd624810
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Zach Amsden <[email protected]>
Gerrit-HasComments: Yes

Reply via email to