Support Utf8View and BinaryView in substring - #10672
Open
1fanwang wants to merge 2 commits into
Open
Conversation
substring() handles Utf8, LargeUtf8, Binary, LargeBinary, FixedSizeBinary and Dictionary, but rejects the view types with "substring does not support type Utf8View". The sibling kernels in this crate already take them: length, bit_length, like and concat_elements all have Utf8View arms. Views carry their own offsets rather than sharing one value buffer, so the byte range is computed per element and the result is built through the view builder. The utf-8 boundary check applies to Utf8View as it does to Utf8; the offset in that error is per element, since there is no shared buffer to index. Tested by comparing the view result against the Utf8 and Binary results over the same inputs, including empty strings, nulls, values longer than the twelve-byte inline limit, zero length, and starts past either end. Signed-off-by: 1fanwang <1fannnw@gmail.com>
Jefffrey
added
the
enhancement
Jefffrey
reviewed
Aug 13, 2026
|
|
||
| #[test] | ||
| fn binary_view_matches_binary() { | ||
| let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")]; |
Contributor
There was a problem hiding this comment.
should add a long one here too
| } | ||
|
|
||
| #[test] | ||
| fn string_view_rejects_an_invalid_char_boundary() { |
Contributor
There was a problem hiding this comment.
do we have a positive case, for slicing on a valid boundary that isnt only ascii?
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Jefffrey
approved these changes
Aug 13, 2026
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.
Which issue does this PR close?
Rationale for this change
substring()takesUtf8,LargeUtf8,Binary,LargeBinary,FixedSizeBinaryandDictionary, but rejects the view types, so a caller holding aStringViewArrayhas to cast toUtf8first and pay for the copy. The other string kernels in this crate already accept them:length,bit_length,likeandconcat_elementsall haveUtf8Viewarms.What changes are included in this PR?
Utf8ViewandBinaryViewarms. Views carry their own offsets rather than sharing one value buffer, so the byte range is computed per element and the result is built through the view builder. The existingDictionaryarm recurses, so a dictionary of views now works too.The utf-8 boundary check applies to
Utf8Viewthe same way it does toUtf8. The offset quoted in that error is per element, because there is no shared buffer to index into.Are these changes tested?
Yes. The view result is compared against the
Utf8andBinaryresult for the same inputs, so the two paths cannot drift: empty strings, nulls, values longer than the twelve-byte inline limit, zero length, lengths past the end, and starts past either end in both directions. Plus the boundary error and a dictionary of views.The cross-check earned its keep — it caught the error offsets differing between the two paths, which is what prompted the note above.
Tests
Removing the two new match arms fails exactly the four new tests, so they cannot pass vacuously: