Zach Amsden has posted comments on this change. Change subject: IMPALA-2020: Inline big number strings ......................................................................
Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/5902/5//COMMIT_MSG Commit Message: PS5, Line 38: 0.83s > I mean run the test a few times and provide something like https://en.wikip Do we have any nice scripts to do that? I'd rather not go to that trouble when all that is left is the obvious win of getting the constants declared in the header and not requiring DecimalUtil::InitMaxUnscaledDecimal() http://gerrit.cloudera.org:8080/#/c/5902/5/be/src/util/decimal-util.h File be/src/util/decimal-util.h: Line 57: template<typename T> > Neither looks done in PS9: private or rename Moot point now - killed the code PS5, Line 58: T > I'm not so sure: it would lose precision there in the low bits. Sadly, other uses GetScaleMultiplier below for exactly this - to get a power of 10 in double. PS5, Line 59: boost:: > It helps me keep track of comments on patches when they are responded to on Duly noted Line 60: return scale == 0 ? 1 : 10 * GetConstScaleMultiplier<T>(scale-1); > Why not just another '?:' operator? moot Line 136: if (__builtin_const_p(scale) && p < 10) return GetConstScaleMultiplier<int32_t>(scale); > Please post the numbers here when you have decided. Wash. The only win was from getting the MAX_UNSCALED_DECIMAL constants inline. -- To view, visit http://gerrit.cloudera.org:8080/5902 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5095a366d914cebb0b64bd434a08dbb55c90ed30 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <zams...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-HasComments: Yes
