Fix cudf::strings::is_fixed_point checking of overflow for decimal32 #9093
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.
While working on
decimal128
support, @codereport found a bug in thecudf::strings::is_fixed_point
logic where a large integer (in a strings column) could return true/valid even though it overflows theRep
type fordecimal32 type
. The gtest values did not include a value that would have shown this error. This PR adds the test string and fixes the logic properly check the overflow condition.The current logic was relying on storing intermediate values into
uint64_t
types so any number that would fit inuint64_t
would not be detected as overflow fordecimal32
. This PR fixes functions to use the input type storage type more to help identify the overflow correctly and to help with specializing fordecimal128
.