[fix](be) Move #include directives outside namespace blocks to avoid ODR violations#62871
Open
csun5285 wants to merge 1 commit intoapache:masterfrom
Open
[fix](be) Move #include directives outside namespace blocks to avoid ODR violations#62871csun5285 wants to merge 1 commit intoapache:masterfrom
csun5285 wants to merge 1 commit intoapache:masterfrom
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
…ODR violations
Five headers/sources had `#include` directives placed inside an outer
`namespace` block. The preprocessor expanded each included header in
the wrong nesting, producing differently-named (and potentially
differently-laid-out) class declarations from what the rest of the build
sees. Header guards then locked-in whichever expansion landed first per
TU, so the linker silently merged ODR-violating definitions and resolved
calls to mismatched vtables / member offsets. The observed symptom was
random SIGSEGV during variant doc-mode read paths
(`CombineMultipleBinaryColumnIterator::_collect_sparse_data_from_buckets`
sorting `std::tuple<string_view,...>`, `BinaryDictPageDecoder::next_batch`
null deref via `VariantDocValueCompactIterator`), but the latent issue
applied to every TU that included these files.
Files fixed:
- `be/src/storage/segment/variant/variant_doc_snpashot_compact_iterator.h`
— Doris header `core/column/column_variant.h` /
`storage/segment/column_reader.h` were included inside
`namespace doris::segment_v2 {`, nesting their `namespace doris { ... }`
declarations as `doris::segment_v2::doris::segment_v2::ColumnIterator`.
This was the source of the variant-mode crashes.
- `be/src/exprs/table_function/vexplode_v2.cpp` — Same pattern with
`core/column/column_struct.h` inside `namespace doris {`.
- `be/src/util/simd/vstring_function.h` — `<x86intrin.h>` included inside
`namespace doris {` (under `#ifdef __AVX2__`); also redundant because
`<immintrin.h>` is already included at file scope.
- `be/src/exec/common/sip_hash.h` — `<cstddef>` included inside
`namespace doris {`; lower impact (mostly typedefs) but still wrong.
- `be/src/util/hash/murmur_hash3.cpp` — `<stdlib.h>` included inside
`namespace doris {` under `#if defined(_MSC_VER)`; dead on
Linux/macOS but corrected for consistency.
In every case the fix is the same: hoist the `#include` to file scope,
above any `namespace` open.
csun5285
force-pushed
the
fix/odr-include-inside-namespace
branch
from
f467497 to
ce443f7
Compare
Contributor
Author
|
run buildall |
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.
Several files placed
#includedirectives inside an outernamespace { ... }block.The preprocessor expands the included header verbatim at that point, so a header that opens its own
namespace doris { namespace segment_v2 { ... } }ends up nested asdoris::segment_v2::doris::segment_v2::ColumnIteratorinstead ofdoris::segment_v2::ColumnIterator.What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)