Zach Amsden has posted comments on this change. Change subject: IMPALA-2020, 4915, 4936: Add rounding for decimal casts ......................................................................
Patch Set 18: (4 comments) Nope, doesn't solve the DCHECK issue. I added a DCHECK that the static cast back to the target type from double equals the actual value of the target type, and it failed. Issue is real, see the updated comment in be/src/runtime/decimal-value.inline.h Floating point is not so fun, and the conversion really does lose precision. Issue is also pre-existing so I'm not trying to fix it now. http://gerrit.cloudera.org:8080/#/c/5951/18/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 1395: { true, 0, 38, 38 }}}, // This is overflowing, but wasn't noticed by the test before > let's just say IMPALA-4964 rather than refer to what the old code did, and Done PS18, Line 1397: related to this > related to what? Done PS18, Line 1398: IMAPALA > IMPALA. also combine to one line Done Line 1635: TestIsNotNull(c.expr, type); > mentioned in the other change -- Done -- 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: 18 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
