Repository navigation
branch-4.2: [fix](be) Fix CONV results for negative string inputs #68984 - #70377
Open
github-actions[bot] wants to merge 1 commit into
Open
github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: CONV('-10', 10, 10) returned 18446744073709551615 while
CONV(-10, 10, 10) returned 18446744073709551606. Converting the string
to
signed output with CONV('-10', 10, -10) or CONV('-10', 2, -10) likewise
returned -1 instead of -10 or -2.
The base-aware string parser added one to UINT64_MAX when it encountered
a negative sign, wrapping the unsigned magnitude limit to zero and
incorrectly reporting overflow. Expand the negative magnitude limit only
for signed integer types, using unsigned arithmetic; keep the full
unsigned magnitude limit when parsing negative strings as uint64_t.
Negative destination bases continue to select signed output, and allowed
base magnitudes remain [2, 36]. Add unsigned parser boundary tests and
CONV
coverage for negative string and integer inputs, signed and unsigned
output, 64-bit limits and overflow, NULLs, all TINYINT base values, all
eight constant/column argument combinations, and constant folding.
Generate the regression output through the standard regression script.
The reported six-expression query now returns
18446744073709551606, 18446744073709551606, -10, -10, -2, -2.
### Release note
Fix incorrect CONV results for negative string inputs, including signed
output selected by a negative destination base.
### Check List (For Author)
- Test:
- ASAN BE build: ./build.sh --be -j32
- Unit Test: ./run-be-ut.sh --run
--filter='MathFunctionTest.conv*:StringToIntWithBase.*' -j32 (10 tests
passed)
- Regression test: ./run-regression-test.sh --run -d
query_p0/sql_functions/math_functions -s test_conv_negative_base_range
-forceGenOut
- Regression test: ./run-regression-test.sh --run -d
query_p0/sql_functions/math_functions -s
test_conv,test_conv_negative_base_range (2 suites passed)
- Manual test: verified the reported six-expression SQL on the rebuilt
worktree cluster
- clang-format 16, clang-tidy on changed lines, build hygiene, and git
diff checks passed
- Behavior changed: Yes, correct negative string conversion results
while preserving existing negative base support
- Does this need documentation: No, this is a bug fix with existing base
semantics preserved
### 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 -->
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
run buildall |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Cherry-picked from #68984