[fix](decimal) fix undefined behaviour of divide by zero when cast string to decimal #26822
Conversation
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
be/src/vec/common/int_exp.h
Outdated
| inline constexpr std::int32_t exp10_i32(int x) { | ||
| return exp_details::get_exp<std::int32_t, 10, 10>(x); | ||
| constexpr inline int exp10_i32(int x) { | ||
| if (x < 0) return 0; |
There was a problem hiding this comment.
warning: statement should be inside braces [readability-braces-around-statements]
| if (x < 0) return 0; | |
| if (x < 0) { return 0; | |
| } |
| if (x < 0) return 0; | ||
| if (x > 9) return std::numeric_limits<int>::max(); | ||
|
|
||
| constexpr int values[] = {1, 10, 100, 1000, 10000, |
There was a problem hiding this comment.
warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
constexpr int values[] = {1, 10, 100, 1000, 10000,
^
be/src/vec/common/int_exp.h
Outdated
| inline constexpr std::int64_t exp10_i64(int x) { | ||
| return exp_details::get_exp<std::int64_t, 10, 19>(x); | ||
| constexpr inline int64_t exp10_i64(int x) { | ||
| if (x < 0) return 0; |
There was a problem hiding this comment.
warning: statement should be inside braces [readability-braces-around-statements]
| if (x < 0) return 0; | |
| if (x < 0) { return 0; | |
| } |
| if (x < 0) return 0; | ||
| if (x > 18) return std::numeric_limits<int64_t>::max(); | ||
|
|
||
| constexpr int64_t values[] = {1LL, |
There was a problem hiding this comment.
warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
constexpr int64_t values[] = {1LL,
^
be/src/vec/common/int_exp.h
Outdated
| inline constexpr __int128 exp10_i128(int x) { | ||
| return exp_details::get_exp<__int128, 10, 39>(x); | ||
| constexpr inline __int128 exp10_i128(int x) { | ||
| if (x < 0) return 0; |
There was a problem hiding this comment.
warning: statement should be inside braces [readability-braces-around-statements]
| if (x < 0) return 0; | |
| if (x < 0) { return 0; | |
| } |
| if (x < 0) return 0; | ||
| if (x > 38) return std::numeric_limits<__int128>::max(); | ||
|
|
||
| constexpr __int128 values[] = { |
There was a problem hiding this comment.
warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
constexpr __int128 values[] = {
^
|
run buildall |
| if (x < 0) { | ||
| return 0; | ||
| } | ||
| if (x > 19) { |
There was a problem hiding this comment.
warning: 19 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
if (x > 19) {
^| if (x < 0) { | ||
| return 0; | ||
| } | ||
| if (x > 9) { |
There was a problem hiding this comment.
warning: 9 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
if (x > 9) {
^| if (x < 0) { | ||
| return 0; | ||
| } | ||
| if (x > 18) { |
There was a problem hiding this comment.
warning: 18 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
if (x > 18) {
^| if (x < 0) { | ||
| return 0; | ||
| } | ||
| if (x > 38) { |
There was a problem hiding this comment.
warning: 38 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
if (x > 38) {
^| if (x < 0) { | ||
| return 0; | ||
| } | ||
| if (x > 76) { |
There was a problem hiding this comment.
warning: 76 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
if (x > 76) {
^
|
TeamCity be ut coverage result: |
|
TeamCity be ut coverage result: |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run p0 |
|
PR approved by anyone and no changes requested. |
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
…ring to decimal (apache#26822) * [fix](decimal) fix undefined behaviour of divide by zero when cast string to decimal * fix format
…ring to decimal (apache#26822) * [fix](decimal) fix undefined behaviour of divide by zero when cast string to decimal * fix format
…ring to decimal (apache#26822) * [fix](decimal) fix undefined behaviour of divide by zero when cast string to decimal * fix format
Proposed changes
Issue Number: close #xxx
For sql
select cast('0.00164999999999998' as decimalv3(9,0));be will coredump:The reason is that function common::exp10_i32 called by get_scale_multiplier(11) will read out-of-range of the exp_table array, which result in Undefined Behaviour, and the if (LIKELY(divisor > 0)) statement if false positive.
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...