You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "1fanwang (via GitHub)" <gi...@apache.org> on 2026/08/13 00:50:52 UTC
[PR] Support Utf8View and BinaryView in substring [arrow-rs]
1fanwang opened a new pull request, #10672:
URL: https://github.com/apache/arrow-rs/pull/10672
# Which issue does this PR close?
- Closes #10671.
# Rationale for this change
`substring()` takes `Utf8`, `LargeUtf8`, `Binary`, `LargeBinary`, `FixedSizeBinary` and `Dictionary`, but rejects the view types, so a caller holding a `StringViewArray` has to cast to `Utf8` first and pay for the copy. The other string kernels in this crate already accept them: `length`, `bit_length`, `like` and `concat_elements` all have `Utf8View` arms.
# What changes are included in this PR?
`Utf8View` and `BinaryView` 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 existing `Dictionary` arm recurses, so a dictionary of views now works too.
The utf-8 boundary check applies to `Utf8View` the same way it does to `Utf8`. 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 `Utf8` and `Binary` result 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.
<details><summary>Tests</summary>
```
$ cargo test -p arrow-string
186 passed; 0 failed
```
Removing the two new match arms fails exactly the four new tests, so they cannot pass vacuously:
```
test substring::tests::string_view_matches_utf8 ... FAILED
test substring::tests::binary_view_matches_binary ... FAILED
test substring::tests::string_view_rejects_an_invalid_char_boundary ... FAILED
test substring::tests::dictionary_of_string_view ... FAILED
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Utf8View and BinaryView in substring [arrow-rs]
Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on code in PR #10672:
URL: https://github.com/apache/arrow-rs/pull/10672#discussion_r3772611637
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
+ #[test]
+ fn string_view_matches_utf8() {
+ let values = vec![
+ Some("hello world"),
+ Some(""),
+ None,
+ Some("a"),
+ Some("this one is definitely longer than twelve bytes"),
+ ];
+ let utf8 = StringArray::from(values.clone());
+ let view = StringViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(0)),
+ (0, Some(5)),
+ (0, Some(1000)),
+ (1, Some(3)),
+ (5, None),
+ (100, Some(2)),
+ (100, None),
+ (-3, None),
+ (-3, Some(2)),
+ (-100, Some(4)),
+ (-100, None),
+ ] {
+ let expected = substring(&utf8, start, length).unwrap();
+ let expected = expected.as_string::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_string_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn binary_view_matches_binary() {
+ let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")];
+ let binary = BinaryArray::from(values.clone());
+ let view = BinaryViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(5)),
+ (2, Some(3)),
+ (-3, None),
+ (100, Some(2)),
+ ] {
+ let expected = substring(&binary, start, length).unwrap();
+ let expected = expected.as_binary::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_binary_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn string_view_rejects_an_invalid_char_boundary() {
Review Comment:
Added in 336f3d5. It cross-checks `héllo wörld` and `日本語` against the Utf8 path on the offsets that are char boundaries in both, and asserts `substring(_, 0, 3)` gives `hé` and `日`.
Writing it caught my own bad offsets: `-4` lands mid-`ö`, and both paths errored identically, which was reassuring.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Utf8View and BinaryView in substring [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #10672:
URL: https://github.com/apache/arrow-rs/pull/10672
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Utf8View and BinaryView in substring [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10672:
URL: https://github.com/apache/arrow-rs/pull/10672#discussion_r3771844234
##########
arrow-string/src/substring.rs:
##########
@@ -247,6 +250,68 @@ fn utf8_bounds(val: &str, start: i64, length: Option<usize>) -> (usize, usize) {
(start_offset, end_offset)
}
+/// Byte range of one element, following the same rules as [`byte_substring`].
+fn view_substring_range(len: usize, start: i64, length: Option<u64>) -> (usize, usize) {
Review Comment:
its confusing to have both `len` and `length`, perhaps rename them to something like `original_length` and `substring_length`
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
+ #[test]
+ fn string_view_matches_utf8() {
+ let values = vec![
+ Some("hello world"),
+ Some(""),
+ None,
+ Some("a"),
+ Some("this one is definitely longer than twelve bytes"),
+ ];
+ let utf8 = StringArray::from(values.clone());
+ let view = StringViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(0)),
+ (0, Some(5)),
+ (0, Some(1000)),
+ (1, Some(3)),
+ (5, None),
+ (100, Some(2)),
+ (100, None),
+ (-3, None),
+ (-3, Some(2)),
+ (-100, Some(4)),
+ (-100, None),
+ ] {
+ let expected = substring(&utf8, start, length).unwrap();
+ let expected = expected.as_string::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_string_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn binary_view_matches_binary() {
+ let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")];
+ let binary = BinaryArray::from(values.clone());
+ let view = BinaryViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(5)),
+ (2, Some(3)),
+ (-3, None),
+ (100, Some(2)),
+ ] {
+ let expected = substring(&binary, start, length).unwrap();
+ let expected = expected.as_binary::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_binary_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn string_view_rejects_an_invalid_char_boundary() {
Review Comment:
do we have a positive case, for slicing on a valid boundary that isnt only ascii?
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
Review Comment:
```suggestion
```
goes without saying
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
+ #[test]
+ fn string_view_matches_utf8() {
+ let values = vec![
+ Some("hello world"),
+ Some(""),
+ None,
+ Some("a"),
+ Some("this one is definitely longer than twelve bytes"),
+ ];
+ let utf8 = StringArray::from(values.clone());
+ let view = StringViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(0)),
+ (0, Some(5)),
+ (0, Some(1000)),
+ (1, Some(3)),
+ (5, None),
+ (100, Some(2)),
+ (100, None),
+ (-3, None),
+ (-3, Some(2)),
+ (-100, Some(4)),
+ (-100, None),
+ ] {
+ let expected = substring(&utf8, start, length).unwrap();
+ let expected = expected.as_string::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_string_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn binary_view_matches_binary() {
+ let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")];
Review Comment:
should add a long one here too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Utf8View and BinaryView in substring [arrow-rs]
Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on code in PR #10672:
URL: https://github.com/apache/arrow-rs/pull/10672#discussion_r3772629917
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
+ #[test]
+ fn string_view_matches_utf8() {
+ let values = vec![
+ Some("hello world"),
+ Some(""),
+ None,
+ Some("a"),
+ Some("this one is definitely longer than twelve bytes"),
+ ];
+ let utf8 = StringArray::from(values.clone());
+ let view = StringViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(0)),
+ (0, Some(5)),
+ (0, Some(1000)),
+ (1, Some(3)),
+ (5, None),
+ (100, Some(2)),
+ (100, None),
+ (-3, None),
+ (-3, Some(2)),
+ (-100, Some(4)),
+ (-100, None),
+ ] {
+ let expected = substring(&utf8, start, length).unwrap();
+ let expected = expected.as_string::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_string_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn binary_view_matches_binary() {
+ let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")];
+ let binary = BinaryArray::from(values.clone());
+ let view = BinaryViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(5)),
+ (2, Some(3)),
+ (-3, None),
+ (100, Some(2)),
+ ] {
+ let expected = substring(&binary, start, length).unwrap();
+ let expected = expected.as_binary::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_binary_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn string_view_rejects_an_invalid_char_boundary() {
Review Comment:
Added in https://github.com/apache/arrow-rs/commit/336f3d55503c9aa491be8cb31fce8b86c4258ae8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Utf8View and BinaryView in substring [arrow-rs]
Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on code in PR #10672:
URL: https://github.com/apache/arrow-rs/pull/10672#discussion_r3772611279
##########
arrow-string/src/substring.rs:
##########
@@ -247,6 +250,68 @@ fn utf8_bounds(val: &str, start: i64, length: Option<usize>) -> (usize, usize) {
(start_offset, end_offset)
}
+/// Byte range of one element, following the same rules as [`byte_substring`].
+fn view_substring_range(len: usize, start: i64, length: Option<u64>) -> (usize, usize) {
Review Comment:
Done in 336f3d5 — `original_length` / `substring_length`.
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
Review Comment:
Done in 336f3d5.
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
+ #[test]
+ fn string_view_matches_utf8() {
+ let values = vec![
+ Some("hello world"),
+ Some(""),
+ None,
+ Some("a"),
+ Some("this one is definitely longer than twelve bytes"),
+ ];
+ let utf8 = StringArray::from(values.clone());
+ let view = StringViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(0)),
+ (0, Some(5)),
+ (0, Some(1000)),
+ (1, Some(3)),
+ (5, None),
+ (100, Some(2)),
+ (100, None),
+ (-3, None),
+ (-3, Some(2)),
+ (-100, Some(4)),
+ (-100, None),
+ ] {
+ let expected = substring(&utf8, start, length).unwrap();
+ let expected = expected.as_string::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_string_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn binary_view_matches_binary() {
+ let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")];
Review Comment:
Done in 336f3d5 — added a 47-byte value so the binary case crosses the inline limit too.
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
+ #[test]
+ fn string_view_matches_utf8() {
+ let values = vec![
+ Some("hello world"),
+ Some(""),
+ None,
+ Some("a"),
+ Some("this one is definitely longer than twelve bytes"),
+ ];
+ let utf8 = StringArray::from(values.clone());
+ let view = StringViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(0)),
+ (0, Some(5)),
+ (0, Some(1000)),
+ (1, Some(3)),
+ (5, None),
+ (100, Some(2)),
+ (100, None),
+ (-3, None),
+ (-3, Some(2)),
+ (-100, Some(4)),
+ (-100, None),
+ ] {
+ let expected = substring(&utf8, start, length).unwrap();
+ let expected = expected.as_string::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_string_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn binary_view_matches_binary() {
+ let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")];
+ let binary = BinaryArray::from(values.clone());
+ let view = BinaryViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(5)),
+ (2, Some(3)),
+ (-3, None),
+ (100, Some(2)),
+ ] {
+ let expected = substring(&binary, start, length).unwrap();
+ let expected = expected.as_binary::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_binary_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn string_view_rejects_an_invalid_char_boundary() {
Review Comment:
Added in 336f3d5. It cross-checks `héllo wörld` and `日本語` against the Utf8 path on the offsets that are char boundaries in both, and asserts `substring(_, 0, 3)` gives `hé` and `日`.
Writing it caught my own bad offsets: `-4` lands mid-`ö`, and both paths errored identically, which was reassuring.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Utf8View and BinaryView in substring [arrow-rs]
Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on code in PR #10672:
URL: https://github.com/apache/arrow-rs/pull/10672#discussion_r3772611529
##########
arrow-string/src/substring.rs:
##########
@@ -1035,4 +1100,90 @@ mod tests {
let expected = BinaryArray::from(vec![Some(expected_bytes)]);
assert_eq!(expected, *actual);
}
+
+ /// The view result has to match the [`DataType::Utf8`] result for the same input.
+ #[test]
+ fn string_view_matches_utf8() {
+ let values = vec![
+ Some("hello world"),
+ Some(""),
+ None,
+ Some("a"),
+ Some("this one is definitely longer than twelve bytes"),
+ ];
+ let utf8 = StringArray::from(values.clone());
+ let view = StringViewArray::from(values);
+
+ for (start, length) in [
+ (0, None),
+ (0, Some(0)),
+ (0, Some(5)),
+ (0, Some(1000)),
+ (1, Some(3)),
+ (5, None),
+ (100, Some(2)),
+ (100, None),
+ (-3, None),
+ (-3, Some(2)),
+ (-100, Some(4)),
+ (-100, None),
+ ] {
+ let expected = substring(&utf8, start, length).unwrap();
+ let expected = expected.as_string::<i32>();
+ let actual = substring(&view, start, length).unwrap();
+ let actual = actual.as_string_view();
+ assert_eq!(
+ expected.iter().collect::<Vec<_>>(),
+ actual.iter().collect::<Vec<_>>(),
+ "start={start} length={length:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn binary_view_matches_binary() {
+ let values: Vec<Option<&[u8]>> = vec![Some(b"hello world"), Some(b""), None, Some(b"abc")];
Review Comment:
Done in 336f3d5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Utf8View and BinaryView in substring [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10672:
URL: https://github.com/apache/arrow-rs/pull/10672#issuecomment-5290587481
thanks @1fanwang
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org