Zach Amsden 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 Done PS5, Line 38: 0.83s > error bars? what do you mean exactly? 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; > Why do you want to convert it to an Rvalue? Just to avoid having to define I just defined it in the .cc - not doing so (and not converting to an rvalue) breaks the debug build, but not the release build, as this does actually get inlined if defined in the header. 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. Done PS5, Line 58: T > Always is_integral? Not necessarily, this would work for float / double as well. PS5, Line 59: boost:: > static_assert is now part of the language, but I'm not sure if it works on I'll try it Line 60: return scale == 0 ? 1 : 10 * GetConstScaleMultiplier<T>(scale-1); > Check for no overflow, if it can be done simply. I think it can be done, not sure how simply. Since this is constexpr, we are pretty constrained in what we can use but I think another static assert could do it. 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 Let's remove it and find out. -- 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
