perf: Optimize split_part for Utf8View#21420
Merged
alamb merged 2 commits intoapache:mainfrom Apr 8, 2026
Merged
Conversation
github-actions
Bot
added
sqllogictest
Contributor
Author
martin-g
reviewed
Apr 7, 2026
metegenez
reviewed
Apr 7, 2026
Contributor
metegenez
left a comment
metegenez
left a comment
There was a problem hiding this comment.
It might be worth adding unit tests for sliced StringViewArray inputs (non-zero offset) and results landing exactly at the 12-byte inline/out-of-line boundary. These are the two spots most likely to surface issues with the unsafe view construction.
Other than that, nice optimization.
| split_nth_finder(s, &finder, delimiter.len(), idx) | ||
| }); | ||
| } else { | ||
| let idx: usize = (position.unsigned_abs() - 1).try_into().map_err(|_| { |
Contributor
There was a problem hiding this comment.
upstream guarantees position will not be zero, so this looks safe.
Contributor
Author
Thanks! Added some tests for those cases. |
martin-g
approved these changes
Apr 7, 2026
alamb
approved these changes
Apr 7, 2026
Contributor
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Looks good to me too -- thanks @neilconway
metegenez
approved these changes
Apr 8, 2026
Contributor
|
Thanks everyone |
Rich-T-kid
pushed a commit
to Rich-T-kid/datafusion
that referenced
this pull request
Apr 21, 2026
## Which issue does this PR close? - Closes apache#21410. ## Rationale for this change When `split_part` is invoked with a `StringViewArray`, we can avoid copying when constructing the return value by instead returning pointers into the view buffers of the input `StringViewArray`. This PR only applies this optimization to the code path for scalar `delimiter` and `position`, because that's the most common usage mode in practice. We could also optimize the array-args case but it didn't seem worth the extra code. Benchmarks (M4 Max): - scalar_utf8view_very_long_parts/pos_first: 102 µs → 68 µs (-33%) - scalar_utf8view_long_parts/pos_middle: 164 µs → 137 µs (-15%) - scalar_utf8_single_char/pos_first: 42.5 µs → 42.9 µs (no change) - scalar_utf8_single_char/pos_middle: 96.5 µs → 99.5 µs (noise) - scalar_utf8_single_char/pos_negative: 48.3 µs → 48.6 µs (no change) - scalar_utf8_multi_char/pos_middle: 132 µs → 132 µs (no change) - scalar_utf8_long_strings/pos_middle: 1.06 ms → 1.08 ms (noise) - array_utf8_single_char/pos_middle: 355 µs → 365 µs (noise) - array_utf8_multi_char/pos_middle: 357 µs → 360 µs (no change) ## What changes are included in this PR? * Implement optimization * Add benchmark that covers this case * Improve SLT test coverage for this code path ## Are these changes tested? Yes. ## Are there any user-facing changes? No.
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.
Which issue does this PR close?
split_partto avoid copying via StringView #21410.Rationale for this change
When
split_partis invoked with aStringViewArray, we can avoid copying when constructing the return value by instead returning pointers into the view buffers of the inputStringViewArray.This PR only applies this optimization to the code path for scalar
delimiterandposition, because that's the most common usage mode in practice. We could also optimize the array-args case but it didn't seem worth the extra code.Benchmarks (M4 Max):
What changes are included in this PR?
Are these changes tested?
Yes.
Are there any user-facing changes?
No.