Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/8833 )
Change subject: IMPALA-6300: Fix decimal modulo overflow ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/8833/2/be/src/runtime/decimal-value.inline.h File be/src/runtime/decimal-value.inline.h: http://gerrit.cloudera.org:8080/#/c/8833/2/be/src/runtime/decimal-value.inline.h@182 PS2, Line 182: 73, 76, 79, 83, 86, 89, 93, 96, 99, 102, 106, 109, 112, 116, 119, 122, 126, 129 }; > Why up to 10^40? Can we really scale that high? I would have thought the It went to a scale of 10^39 before. Changed it so that we go up to a scale of 38. http://gerrit.cloudera.org:8080/#/c/8833/2/be/src/runtime/decimal-value.inline.h@613 PS2, Line 613: *y_scaled = detail::SafeMultiply<RESULT_T>(y.value(), scale_factor, false); > I checked and this is was the only usage of GetScaleQuotient outside of tes Removed the function. Also got rid of the overflow check in the benchmark function because we now assume that AdjustToSameScale() should not overflow. I think the whole benchmark file needs to be updated (or removed if nobody uses it). -- To view, visit http://gerrit.cloudera.org:8080/8833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686 Gerrit-Change-Number: 8833 Gerrit-PatchSet: 2 Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-Comment-Date: Thu, 21 Dec 2017 20:49:13 +0000 Gerrit-HasComments: Yes
