Skip to content

perf(substr_index): speed up scalar and Utf8View#21754

Merged
alamb merged 2 commits intoapache:mainfrom
kumarUjjawal:perf/substr_index_utfview
Apr 25, 2026
Merged

perf(substr_index): speed up scalar and Utf8View#21754
alamb merged 2 commits intoapache:mainfrom
kumarUjjawal:perf/substr_index_utfview

Conversation

@kumarUjjawal
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

substr_index is part of the Utf8View epic. This function was still missing the Utf8View optimization path, and it also did extra work when delimiter and count were constant scalars.

This PR adds the same kind of optimization used in similar string work and includes benchmark coverage.

What changes are included in this PR?

  • Return the input string type directly, including Utf8View.
  • Add a zero-copy Utf8View path for substr_index.
  • Add a scalar fast path when delimiter and count are constant.
  • Keep the hot path specialized after checking a cleaner shared-helper version and benchmarking it.
  • Add SQL coverage for Utf8View type and value results.
  • Add unit tests for the scalar fast path, sliced Utf8View arrays, and the unchanged original-view case.
  • Extend the benchmark to cover Utf8, Utf8View, array arguments, scalar delimiter/count, and positive and negative counts.

Are these changes tested?

Yes

Are there any user-facing changes?

No

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Apr 21, 2026
@kumarUjjawal
Copy link
Copy Markdown
Contributor Author

cc @neilconway since you are working on this area.

@neilconway
Copy link
Copy Markdown
Contributor

@kumarUjjawal Thanks for working on this! Can you post some benchmark data, please?

Copy link
Copy Markdown
Contributor

@neilconway neilconway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise lgtm!

count_array,
GenericStringBuilder::<i32>::with_capacity(
string_array.len(),
string_array.value_data().len(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below, this over-allocates the buffer capacity for sliced arrays.

@kumarUjjawal
Copy link
Copy Markdown
Contributor Author

@kumarUjjawal Thanks for working on this! Can you post some benchmark data, please?

Case Before After Result
Utf8 scalar, single delimiter, count = 1 20.18 us 11.29 us 1.79x faster
Utf8 scalar, long delimiter, count = 1 39.82 us 13.69 us 2.91x faster
Utf8View array, single delimiter 26.64 us 11.77 us 2.26x faster
Utf8View scalar, long delimiter, count = 1 40.59 us 12.88 us 3.15x faster
Utf8View array, long delimiter 52.47 us 46.28 us 1.13x faster

@kumarUjjawal
Copy link
Copy Markdown
Contributor Author

Thanks @neilconway

Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alamb alamb added this pull request to the merge queue Apr 25, 2026
Merged via the queue into apache:main with commit aab4263 Apr 25, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants