[BugFix] Fix ASAN crash for BOOLEAN nested in Avro complex-type columns#76041
Conversation
BooleanColumn is FixedLengthColumn<uint8_t>, but the nested (non-adaptive) add_nullable_column dispatched TYPE_BOOLEAN through add_nullable_numeric_column<int8_t>. add_numeric_column then down_cast the uint8 column to FixedLengthColumn<int8_t>*, tripping the dynamic_cast assert (gutil/casts.h) under ASAN/DEBUG whenever a BOOLEAN is nested inside a native STRUCT/MAP/ARRAY target column of an Avro routine load (StarRocksTest#11522). The adaptive path (add_adpative_nullable_column, used for top-level columns) already dispatched TYPE_BOOLEAN through <uint8_t>, which is why only complex types crashed. Dispatch the nested path through <uint8_t> as well. Add regression test AvroScannerTest.test_struct_with_boolean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GN8hHLLGz7o25mvSJhwqXr
|
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ibility test_struct_with_boolean depends on native STRUCT target support (PR StarRocks#74901, main only), so it cannot cherry-pick to release branches that lack it. Replace it with test_array_with_boolean, which drives the same add_nullable_column TYPE_BOOLEAN dispatch (the fixed line) and whose ARRAY path is present on all release branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GN8hHLLGz7o25mvSJhwqXr
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 1 / 1 (100.00%) file detail
|
|
@Mergifyio backport branch-4.1 |
|
@Mergifyio backport branch-4.0 |
✅ Backports have been createdDetails
|
✅ Backports have been createdDetails
Cherry-pick of bed4395 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Why I'm doing:
Fix the daily ASAN crash reported in StarRocks/StarRocksTest#11522:
It aborts BE when an Avro routine load targets a native complex-type column (STRUCT/MAP/ARRAY) that contains a
BOOLEANfield/element/value.What I'm doing:
BooleanColumnisFixedLengthColumn<uint8_t>, but the nested (non-adaptive)add_nullable_columninbe/src/formats/avro/nullable_column.cppdispatchedTYPE_BOOLEANthroughadd_nullable_numeric_column<int8_t>.add_numeric_column<int8_t>thendown_casts the uint8 column toFixedLengthColumn<int8_t>*, which trips thedynamic_castassert under ASAN/DEBUG.The adaptive path (
add_adpative_nullable_column, used for top-level columns) already dispatchedTYPE_BOOLEANthrough the correct<uint8_t>— which is why only complex types crashed and a plain top-levelBOOLEANcolumn was fine. The fix dispatches the nested path through<uint8_t>too.add_numeric_column<uint8_t>is already explicitly instantiated, so no other change is needed.Added regression test
AvroScannerTest.test_struct_with_boolean; the fullAvroScannerTestsuite (26 tests) passes on a clean ASAN build.Notes:
int8_t/uint8_tare layout-identical, so for valid boolean data this is an ASAN/DEBUG crash rather than a release data-correctness issue.Fixes StarRocks/StarRocksTest#11522
What type of PR is this:
Does this PR entail a change in behavior?
Checklist:
Bugfix cherry-pick branch check: