Jim Apple has posted comments on this change. Change subject: IMPALA-2020: Inline big number strings ......................................................................
Patch Set 5: (8 comments) http://gerrit.cloudera.org:8080/#/c/5902/5//COMMIT_MSG Commit Message: PS5, Line 27: Query: select sum(cast(cast(l_extendedprice as double) as : decimal(14,2))) from tpch10_parquet.lineitem : Query submitted at: 2017-02-18 01:23:54 (Coordinator: : http://impala-dev:25000) : Query progress can be monitored at: : http://impala-dev:25000/query_plan?query_id=f0410a35e981a86b:e6d9207000000000 elide PS5, Line 38: 0.83s error bars? http://gerrit.cloudera.org:8080/#/c/5902/5/be/src/exprs/expr-value.h File be/src/exprs/expr-value.h: Line 177: decimal4_val = +DecimalUtil::MAX_UNSCALED_DECIMAL4; > Unary plus necessary to convert to an Rvalue Why do you want to convert it to an Rvalue? Just to avoid having to define it in a .cc file? If so, does defining (not declaring or initializing) in a .cc file slow down performance? Also, add that comment in the file for future readers. 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> Describe what it does. Maybe call it Pow10 and call the parameter exponent? private: PS5, Line 58: T Always is_integral? PS5, Line 59: boost:: static_assert is now part of the language, but I'm not sure if it works on parameters, since parameters can't be constexpr. Line 60: return scale == 0 ? 1 : 10 * GetConstScaleMultiplier<T>(scale-1); Check for no overflow, if it can be done simply. Line 136: if (__builtin_const_p(scale) && p < 10) return GetConstScaleMultiplier<int32_t>(scale); Does this by itself speed anything up? If scale is constant at compile time, since this is inlined, won't it just inline to the right value? https://godbolt.org/g/Tlfl8l -- 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