Zach Amsden has posted comments on this change. Change subject: IMPALA-2020: Add rounding for decimal casts ......................................................................
Patch Set 10: (1 comment) I am trying to get the DCHECK_EQ to work with int128_t, for which we do provide a definition, but gcc keeps complaining about an ambiguous overload. http://gerrit.cloudera.org:8080/#/c/5951/10/be/src/runtime/decimal-value.inline.h File be/src/runtime/decimal-value.inline.h: PS10, Line 38: if (round) { : // Decimal V2 behavior : : // Multiply the double by the scale. : // : // TODO: this could be made more precise by doing the computation in : // 64 bit with 128 bit immediate result instead of doing an additional : // multiply in 53-bit double precision floating point : : d *= DecimalUtil::GetScaleMultiplier<double>(scale); : d = std::round(d); : const T max_value = DecimalUtil::GetScaleMultiplier<T>(precision); : if (abs(d) >= max_value) { : *overflow = true; : return DecimalValue(); : } : : // Return the rounded integer part. : return DecimalValue(static_cast<T>(d)); : } else { : // TODO: IMPALA-4924: remove DECIMAL V1 code : : // Check overflow. : const T max_value = DecimalUtil::GetScaleMultiplier<T>(precision - scale); : if (abs(d) >= max_value) { : *overflow = true; : return DecimalValue(); : } : : // Multiply the double by the scale. : d *= DecimalUtil::GetScaleMultiplier<double>(scale); : : // Truncate and just take the integer part. : return DecimalValue(static_cast<T>(d)); : } > If it won't break DECIMAL_V1, may as well merge the two paths and add the t It looks like it will work and it doesn't change the speed. -- 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: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
