impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zach Amsden (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4813: Round on divide and multiply
Date Fri, 24 Feb 2017 23:49:30 GMT
Zach Amsden has posted comments on this change.

Change subject: IMPALA-4813: Round on divide and multiply

Patch Set 2:

File be/src/exprs/

PS2, Line 682: ROUND
> GetConstFnAttr() is just as free as a constant 'false'.  The call goes away
Yeah, I guess that makes sense.  Is there another pass that removes unused constants after
File be/src/runtime/decimal-value.h:

Line 124:       int result_scale, bool* overflow) const {
> If they are only used by tests, I agree killing them would be nice.
Attempting to do so
File be/src/runtime/decimal-value.inline.h:

Line 153:   DCHECK_EQ(round, false);
> but what i'm saying is we should actually pass the correct value of 'round'

Line 235:   if (UNLIKELY(delta_scale != 0)) {
Multiply appears to be buggy.  If we ever need to chop the scale, then results that shouldn't
actually overflow after reduction can end up overflowing anyway.

For example, this query overflows despite the fact that the result obviously does not:

 select typeof(t.v), t.v from (select cast(0.001 as decimal(9,4)) * cast(.1 as decimal(38,38))as
v) t;

To view, visit
To unsubscribe, visit

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

View raw message