[refine](column) strong type array and map offsets#63678
Merged
Mryange merged 1 commit intoJun 1, 2026
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
|
OpenCode automated review failed and did not complete. Error: Review step was skipped (possibly timeout or cancelled) Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
TPC-H: Total hot run time: 32194 ms |
Contributor
TPC-DS: Total hot run time: 173195 ms |
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31480 ms |
Contributor
TPC-DS: Total hot run time: 172542 ms |
Contributor
Author
|
run buildall |
Mryange
force-pushed
the
refine-array-map-strong-type-offset
branch
from
26028c3 to
072a404
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31657 ms |
Contributor
TPC-DS: Total hot run time: 171953 ms |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31703 ms |
Contributor
TPC-DS: Total hot run time: 170872 ms |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 31989 ms |
Contributor
TPC-DS: Total hot run time: 171928 ms |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Mryange
force-pushed
the
refine-array-map-strong-type-offset
branch
from
a66ed0f to
e11b51c
Compare
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.
Review result: no blocking issues found in the reviewable PR diff.
Critical checkpoint conclusions:
- Goal/test: The PR focuses on strongly typing ColumnArray/ColumnMap offset subcolumns and removing redundant casts. Existing column unit tests are updated; no new behavior-specific test is required for this refactor, though CI should still run BE UTs for the touched column paths.
- Scope/minimality: The reviewable GitHub diff is focused on Array/Map offset ownership/accessors, deserialize/write-back paths, and direct callers.
- Concurrency: No new shared concurrent state or locks are introduced in the reviewable diff.
- Lifecycle/COW: The typed WrappedPtr conversions and defer-based generic callback bridging follow the existing ColumnNullable pattern; storage reader write-back keeps offsets restored on early return.
- Configuration/incompatibility: No new config, storage format, or wire-format changes in the reviewable diff.
- Parallel paths: Array and Map paths are both updated; function/lambda callers using offsets were adjusted consistently.
- Error handling: Existing invariant checks are preserved via typed asserts/casts; no ignored Status introduced.
- Memory/performance: No new large untracked allocations; the change removes repeated casts and should not add hot-path overhead.
- Observability: No new runtime behavior requiring logs or metrics.
- Data correctness/versioning/MoW: No transaction, version visibility, delete bitmap, or MoW write-path logic is changed.
User focus: no additional user-provided review focus was present.
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 31636 ms |
Contributor
TPC-DS: Total hot run time: 173041 ms |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
HappenLee
reviewed
Jun 1, 2026
| const ColumnPtr& get_offsets_ptr() const { return offsets; } | ||
| ColumnPtr& get_offsets_ptr() { return offsets; } | ||
| const ColumnOffsets::Ptr& get_offsets_ptr() const { | ||
| return static_cast<const ColumnOffsets::Ptr&>(offsets); |
Contributor
There was a problem hiding this comment.
这里需要显式static_cast吗？我感觉不需要啊，反而看起来很复杂
Contributor
Author
There was a problem hiding this comment.
这个chameleon_ptr 和 Ptr不是同一个类型。
template <typename T>
class chameleon_ptr {
private:
immutable_ptr<T> value;
operator const immutable_ptr<T>&() const { return value; }
operator immutable_ptr<T>&() { return value; }
yiguolei
approved these changes
Jun 1, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by anyone and no changes requested. |
zhaorongsheng
pushed a commit
to zhaorongsheng/doris
that referenced
this pull request
Jun 4, 2026
### What problem does this PR solve? Array and map offset subcolumns were still stored and exposed through generic IColumn pointers, so callers had to repeatedly downcast them back to their concrete offset column types. Root cause: the ownership, COW mutation, deserialization, and segment reader write-back paths still treated these offsets as generic subcolumns, which left the strong-typing change incomplete and kept redundant same-type assert_cast usage in affected callers and tests. This change promotes ColumnArray offsets and ColumnMap offsets_column to typed wrapped pointers, updates the typed accessors and generic write-back paths to preserve the concrete offset column type, and removes the now-redundant casts in array functions, lambda functions, and unit tests. It also cleans up one redundant nullable null-map cast uncovered during the BE UT compile. ### Release note None
16 tasks
yiguolei
pushed a commit
that referenced
this pull request
Jun 30, 2026
### What problem does this PR solve? Related PR: #63491, #63678, #63946, #58092 Problem Summary: Backport the column strong-typing refinements for branch-4.1: use strong typed nullable null maps, array offsets, and map offsets, and apply the column check helper for type casts. Branch-4.1 was missing the COW/Column cast helper introduced by #58092, so this also ports the minimal helper API needed by the picked changes and adjusts an old array distance code path to use the already strong typed offsets pointer. ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] 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. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#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 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
4 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?
Array and map offset subcolumns were still stored and exposed through generic IColumn pointers, so callers had to repeatedly downcast them back to their concrete offset column types. Root cause: the ownership, COW mutation, deserialization, and segment reader write-back paths still treated these offsets as generic subcolumns, which left the strong-typing change incomplete and kept redundant same-type assert_cast usage in affected callers and tests. This change promotes ColumnArray offsets and ColumnMap offsets_column to typed wrapped pointers, updates the typed accessors and generic write-back paths to preserve the concrete offset column type, and removes the now-redundant casts in array functions, lambda functions, and unit tests. It also cleans up one redundant nullable null-map cast uncovered during the BE UT compile.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)