impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-2020: Inline big number strings
Date Sat, 18 Feb 2017 03:12:37 GMT
Jim Apple has posted comments on this change.

Change subject: IMPALA-2020: Inline big number strings

Patch Set 5:

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

PS5, Line 38: 0.83s
error bars?
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.
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?


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?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I5095a366d914cebb0b64bd434a08dbb55c90ed30
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Jim Apple <>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden <>
Gerrit-HasComments: Yes

View raw message