Zach Amsden has posted comments on this change. Change subject: IMPALA-4813: Round on divide and multiply ......................................................................
Patch Set 3: Note to reviewers - this diff is only partially ready for review. I wanted to send it anyway to gather feedback on whether the API cleanup in decimal-value-inline.h looks good. I think it vastly de-clutters the API to move all of the test related stuff into the actual test. The test also gets a lot of improvement as a result of the flexibility of specifying any combination of precision / scale without defining a new ColumnType, and the reviewer and future test writers benefit from not having to refer back hundreds of lines to figure out what types t1, t2, t3 actually represent. What is broken right now is the multiply code - the sketch of the non-overflowing implementation is only partial and not fully correct. This means there are test failures as well. Plan for this diff is to undo the multiply change, and likely the multiply front-end change as well, to isolate the effect of any typos that may have been introduced in the unit test conversion. Likely the best thing to do is isolate the multiply change out into a separate diff. -- To view, visit http://gerrit.cloudera.org:8080/6132 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie6bfcbe37555b74598d409c6f84f06b0ae5c4312 Gerrit-PatchSet: 3 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: Zach Amsden <[email protected]> Gerrit-HasComments: No
