Zach Amsden has posted comments on this change. Change subject: IMPALA-4936 and IMPALA-4915: Fix decimal overflow test ......................................................................
Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/6068/2/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: PS2, Line 1397: true > I am missing something too... was the test broken before? I filed JIRA-4964 to investigate. Test was broken before and seems to have uncovered a bug with modulus. http://gerrit.cloudera.org:8080/#/c/6068/2/be/src/runtime/decimal-value.inline.h File be/src/runtime/decimal-value.inline.h: PS2, Line 83: abs > I think we should be consistent and use std::abs() at least in this entire I tried that and it actually turned out to be a bug. The places where std::abs and abs are correct turned out to be a lot more subtle than expected. std::abs selects the correct version for floating point but the wrong version for integers as abs(INT_MIN) is undefined. -- To view, visit http://gerrit.cloudera.org:8080/6068 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8de7440a585c1d3d937fcbb435b9ead77e7b5a63 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
