impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5017: Error on decimal overflow
Date Fri, 03 Nov 2017 23:21:08 GMT
Tim Armstrong has posted comments on this change. ( )

Change subject: IMPALA-5017: Error on decimal overflow

Patch Set 1:

Commit Message:
PS1, Line 12: In this patch, the beharior is changed so that an error is produced in
> typo: beharior should be behavior
I'll also accept behaviour ;)
PS1, Line 27:   select avg(dec_38_19) from decimal_tbl
I guess these regressions occur regardless of the width of the input decimal, rigth?
File be/src/exprs/
PS1, Line 413:           abs(avg->sum_val16) > DecimalUtil::MAX_UNSCALED_DECIMAL16 -
abs(src.val4))) {
This isn't correct if sum_val16 and src.val* have opposite sign, is it?
PS1, Line 420:           abs(avg->sum_val16) > DecimalUtil::MAX_UNSCALED_DECIMAL16 -
abs(src.val8))) {
What do you think about only checking for overflow of the integer type at this point and checking
for whether it fits in the decimal type during finalisation? Might be cheaper.

We could also probably do a cheaper initial check of sum_val16 versus as constant in the 4
and 8 byte cases since they can't possible overflow unless the sum is already quite large.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
Gerrit-Change-Number: 8404
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky <>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Sherman <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Taras Bobrovytsky <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: Zach Amsden <>
Gerrit-Comment-Date: Fri, 03 Nov 2017 23:21:08 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message