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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
