YanzhiJin5 opened a new pull request, #66832:
URL: https://github.com/apache/doris/pull/66832
### What problem does this PR solve?
Issue Number: #63609
Related PR: N/A
Problem Summary:
Persisted dictionary pages may contain invalid signed `int32_t` dictionary
codes. `BinaryDictPageDecoder` previously allowed negative or out-of-range
codes to reach downstream dictionary handling without enforcing:
```text
0 <= code < dictionary_size
```
This could expose invalid dictionary state to downstream consumers instead
of returning a corruption error at the storage decoding boundary.
This PR validates decoded dictionary codes before they become caller-visible:
* `next_batch()` validates the complete decoded batch before inserting
normal dictionary output, resolving `only_read_offsets`, updating `*n`, or
advancing the decoder cursor.
* `read_by_rowids()` gathers and validates all selected codes before
producing caller-visible output or updating `*n`.
* Negative codes and codes greater than or equal to the dictionary size now
return `Status::Corruption`.
* The validation uses a signedness-safe bounds check.
* Valid dictionary-code behavior remains unchanged.
The regression test covers:
* negative dictionary code (`-1`);
* dictionary code equal to `dictionary_size`;
* `next_batch()`;
* `read_by_rowids()`;
* normal dictionary output;
* `only_read_offsets`;
* unchanged destination/count/cursor state when corruption is detected;
* valid-input behavior for all covered paths.
The regression test was verified RED → GREEN: it fails without the
production validation and passes after the validation is restored.
ASAN unit test:
```bash
BUILD_TYPE_UT=ASAN ./run-be-ut.sh --run -j2 \
--filter='BinaryDictPageTest.RejectInvalidDictionaryCodes'
```
Result: **PASS, 1/1**.
Additional checks:
* `build-support/clang-format.sh`: PASS
* `build-support/check-format.sh`: PASS
* `git diff --check origin/master...HEAD`: PASS
* clang-tidy was attempted but was inconclusive due to
pre-existing/toolchain diagnostics; no new diagnostic was observed on the newly
added validation or regression-test lines.
The validation adds one additional sequential O(n) pass over decoded
dictionary codes for valid data. No directly applicable `BinaryDictPageDecoder`
benchmark is currently available, so the performance impact has not been
quantitatively established.
This addresses a matching dictionary-decoder robustness issue found while
investigating the reported crash. The original production segment was not
available for exact reproduction, so this PR does not claim reproduction of the
exact production crash.
### Release note
None
### Check List (For Author)
* Test <!-- At least one of them must be included. -->
* [x] Regression test
* [x] 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.
* [x] Yes. Invalid persisted dictionary codes are now rejected with
`Status::Corruption` at the decoder boundary instead of being propagated to
downstream dictionary handling.
* Does this need documentation?
* [x] No.
* [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/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 -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
