Dan Hecht has posted comments on this change. Change subject: IMPALA-2020, 4915, 4936: Add rounding for decimal casts ......................................................................
Patch Set 18: (4 comments) Does this change solve the DCHECK you were hitting running expr-test? 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 why not combine both lines (we have the same bug in both v1 and v2)? PS18, Line 1397: related to this related to what? PS18, Line 1398: IMAPALA IMPALA. also combine to one line Line 1635: TestIsNotNull(c.expr, type); mentioned in the other change -- thanks for finding that. we should probably add EXPECT_EQ(result, StringParser::PARSE_SUCCESS); to the end of TestDecimalValue(). What's happening is the decimal string parser is failing (when parsing "NULL"), but when it does that it happens to return a DecimalValue with value()=0. -- 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
