[fix](be) Fix macOS BE compilation errors#65783
Open
morningman wants to merge 1 commit into
Open
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Clang 20 fails the macOS BE build because the Linux-only PHDR cache state is unused on macOS and Parquet boolean column-index values are represented by std::vector<bool> proxy references. Restrict the PHDR state field to supported Linux builds and materialize Parquet page bounds as their physical C++ value types before decoding.
### Release note
None
### Check List (For Author)
- Test: Manual test
- sh build.sh --be
- Behavior changed: No
- Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No blocking findings. Review opinion: approve.
Critical checkpoint conclusions:
- Goal and proof: The change addresses the two stated Clang 20/macOS compilation failures. The PHDR state member now exists only where its constructor/destructor use it, and Parquet page-index bounds are materialized as Arrow's declared physical C++ type before decoded-field access, removing the
std::vector<bool>proxy. The code accomplishes the stated goal. - Scope and clarity: The four changed lines are small, focused, and reuse the existing platform predicate and Parquet native-type contract. No unrelated behavior or interface is changed.
- Concurrency and lifecycle: The Linux-glibc PHDR path still saves/restores the thread-local cache flag, including nested scopes. Linux TSan, Linux musl, and non-Linux paths keep the existing no-op special members. The header and implementation predicates match exactly; the Parquet values are synchronous stack locals with no shared-state or ownership change. No lock-order, deadlock, static-initialization, or resource-lifetime issue was found.
- Compatibility and parallel paths: Enabled PHDR builds retain their existing layout and behavior; disabled builds use an internal empty RAII object with no ABI boundary. The Parquet helper is instantiated only for boolean, int32, int64, float, and double; binary types use their separate owning paths. Footer statistics, timestamp monotonicity, NaN/inverted-bound fallback, page-range intersection, and residual filtering semantics remain unchanged. There is no storage-format, protocol, FE-BE, rolling-upgrade, config, transaction, persistence, or data-write change.
- Error handling and data correctness: No
Statusor exception is discarded by the patch. Unsupported or unsafe page-index metadata still conservatively keeps candidate ranges, so the change cannot introduce false page pruning. - Tests and results: Existing PHDR tests cover enabled-path save/restore and cache refresh, while existing Parquet tests cover numeric page-index decoding, NaN/inverted bounds, intersections, skip plans, and unavailable-index fallback. A dedicated Boolean ColumnIndex runtime assertion is absent, but the production translation unit instantiates that template and the fix is specifically a compile-portability correction. Per the review-run instructions, I did not run local builds or tests. The current macOS BE UT workflow failed before compilation because the runner selected JDK 25 instead of the required JDK 17, so that red job neither validates nor contradicts this patch.
- Performance and observability: Two trivial scalar copies per inspected page add no allocation, I/O, locking, or meaningful hot-path cost. No new logging or metrics are warranted.
- User focus: No additional focus points were supplied; the full PR and its upstream/downstream paths were reviewed.
No actionable correctness, build-configuration, lifecycle, compatibility, performance, or coverage defect was found.
Contributor
TPC-H: Total hot run time: 29768 ms |
Contributor
TPC-DS: Total hot run time: 178889 ms |
Contributor
ClickBench: Total hot run time: 25.31 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
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
2 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?
Issue Number: None
Related PR: None
Problem Summary: Clang 20 fails the macOS BE build because the Linux-only PHDR cache state is unused on macOS and Parquet boolean column-index values are represented by std::vector proxy references. Restrict the PHDR state field to supported Linux builds and materialize Parquet page bounds as their physical C++ value types before decoding.