[refactor](be) Use ColumnArrayView in array functions#64215
Open
Mryange wants to merge 1 commit into
Open
Conversation
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary: Refactor array_index, arrays_overlap, and array_remove to read const array inputs through ColumnArrayView instead of expanding const columns with convert_to_full_column_if_const. The dispatch path now uses dispatch_switch_all as the single primitive-type dispatch, including string types, and preserves nullable array semantics through the view helpers.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- sh build.sh --be -j48
- sh run-be-ut.sh --run --filter="function_array_index_test.*:function_arrays_overlap_test.*:function_array_remove_test.*"
- Behavior changed: No
- Does this need documentation: No
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.
Review summary
No blocking issues found in the PR. The refactor keeps the affected functions on the same scalar/string-family paths while replacing const materialization with ColumnArrayView/ColumnView access.
Critical checkpoints:
- Goal and tests: The change addresses const array/scalar handling for array_contains/array_position/countequal, array_remove, and arrays_overlap. Unit tests were added for const arguments. I attempted the targeted BE UT command with bash, but the first-time dependency setup timed out before tests ran in this runner.
- Scope and clarity: The implementation is focused on the three array function headers and related unit tests.
- Concurrency and lifecycle: No new shared state, threads, locks, static initialization, or special ownership lifecycle were introduced.
- Configuration and compatibility: No configs, protocol/storage formats, or persisted metadata are changed.
- Parallel paths: The three modified array-function paths consistently use the new view helper. I checked string-family dispatch and scalar dispatch coverage.
- Error handling and data correctness: Existing Status/exception behavior is preserved for unsupported types; nullable outer-array semantics remain represented in the result null maps. Data visibility/transaction/MoW concerns are not applicable.
- Memory and performance: No untracked long-lived memory ownership is introduced. The main tradeoff is per-element view access for nullable checks, but the change removes const-column materialization and is acceptable for this refactor.
- Observability: No new observability is required for these local expression execution changes.
User focus: No additional user-provided review focus was present.
Contributor
Author
|
run buildall |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29328 ms |
Contributor
TPC-DS: Total hot run time: 169381 ms |
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?
Problem Summary: Refactor array_index, arrays_overlap, and array_remove to read const array inputs through ColumnArrayView instead of expanding const columns with convert_to_full_column_if_const. The dispatch path now uses dispatch_switch_all as the single primitive-type dispatch, including string types, and preserves nullable array semantics through the view helpers.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)