[fix](core) Fix format round would core under boundary conditions. #53660
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 34069 ms |
TPC-DS: Total hot run time: 190539 ms |
ClickBench: Total hot run time: 32.68 s |
zclllyybb
reviewed
Jul 21, 2025
| // Find the position of the decimal point | ||
| size_t dot_pos = formatted_num.find('.'); | ||
| if (dot_pos == std::string::npos) { | ||
| dot_pos = formatted_num.size(); // 没有小数点 |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 34217 ms |
TPC-DS: Total hot run time: 190515 ms |
ClickBench: Total hot run time: 32.81 s |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 34710 ms |
TPC-DS: Total hot run time: 192108 ms |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 33826 ms |
TPC-DS: Total hot run time: 187995 ms |
ClickBench: Total hot run time: 32.44 s |
github-actions
bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
| } | ||
|
|
||
| // Handle the integer part | ||
| int start = (formatted_num[0] == '-') ? 1 : 0; |
Contributor
There was a problem hiding this comment.
Will formatted_num be empty?
Contributor
Author
There was a problem hiding this comment.
这个formatted_num是fmt::format("{:.{}f}", value, decimal_places)的结果。
这个已经判断是一个正规的数了，变成字符串一定不为空，我加一个DCHECK吧。
BiteTheDDDDt
approved these changes
Jul 22, 2025
Mryange
added a commit
to Mryange/doris
that referenced
this pull request
Jul 24, 2025
…pache#53660) my_double_round did not handle NaN values correctly. For example, when dec is very large and value is 0, there will be a case of 0 * inf, resulting in a NaN value. do_format_round assumes the input is always a valid double value, which causes a core dump when a NaN is passed in. Additionally, (value.size() - (is_positive ? (decimal_places + 2) : (decimal_places + 3))) / 3; This code does not account for the situation where value.size() is 0. Currently, a custom add_thousands_separator function is implemented. If the fmt library is upgraded in the future, we should use fmt to add thousands separators.
Mryange
added a commit
to Mryange/doris
that referenced
this pull request
Jul 24, 2025
…pache#53660) my_double_round did not handle NaN values correctly. For example, when dec is very large and value is 0, there will be a case of 0 * inf, resulting in a NaN value. do_format_round assumes the input is always a valid double value, which causes a core dump when a NaN is passed in. Additionally, (value.size() - (is_positive ? (decimal_places + 2) : (decimal_places + 3))) / 3; This code does not account for the situation where value.size() is 0. Currently, a custom add_thousands_separator function is implemented. If the fmt library is upgraded in the future, we should use fmt to add thousands separators.
This was referenced Jul 24, 2025
w41ter
pushed a commit
to w41ter/incubator-doris
that referenced
this pull request
Jul 30, 2025
…pache#53660) my_double_round did not handle NaN values correctly. For example, when dec is very large and value is 0, there will be a case of 0 * inf, resulting in a NaN value. do_format_round assumes the input is always a valid double value, which causes a core dump when a NaN is passed in. Additionally, (value.size() - (is_positive ? (decimal_places + 2) : (decimal_places + 3))) / 3; This code does not account for the situation where value.size() is 0. Currently, a custom add_thousands_separator function is implemented. If the fmt library is upgraded in the future, we should use fmt to add thousands separators.
dataroaring
pushed a commit
that referenced
this pull request
Aug 1, 2025
…ons (#53855) …53660) #53660 my_double_round did not handle NaN values correctly. For example, when dec is very large and value is 0, there will be a case of 0 * inf, resulting in a NaN value. do_format_round assumes the input is always a valid double value, which causes a core dump when a NaN is passed in. Additionally, (value.size() - (is_positive ? (decimal_places + 2) : (decimal_places + 3))) / 3; This code does not account for the situation where value.size() is 0. Currently, a custom add_thousands_separator function is implemented. If the fmt library is upgraded in the future, we should use fmt to add thousands separators. ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
my_double_round did not handle NaN values correctly. For example, when dec is very large and value is 0, there will be a case of 0 * inf, resulting in a NaN value.
do_format_round assumes the input is always a valid double value, which causes a core dump when a NaN is passed in.
Additionally,
(value.size() - (is_positive ? (decimal_places + 2) : (decimal_places + 3))) / 3;
This code does not account for the situation where value.size() is 0.
Currently, a custom add_thousands_separator function is implemented. If the fmt library is upgraded in the future, we should use fmt to add thousands separators.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)