neilconway commented on code in PR #11323:
URL: https://github.com/apache/arrow-rs/pull/11323#discussion_r4158567132
##########
arrow-data/src/data.rs:
##########
@@ -1707,26 +1720,32 @@ impl ArrayData {
}
/// Ensures that all strings formed by the offsets in `buffers[0]`
- /// into `buffers[1]` are valid utf8 sequences
+ /// into `buffers[1]` are valid utf8 sequences. Bytes of `buffers[1]`
+ /// outside the range of the offsets are not checked.
fn validate_utf8<T>(&self) -> Result<(), ArrowError>
where
T: ArrowNativeType + TryInto<usize> + num_traits::Num +
std::fmt::Display,
{
let values_buffer = &self.buffer_at(1)?.as_slice();
- if let Ok(values_str) = std::str::from_utf8(values_buffer) {
- // Validate Offsets are correct
+ let offsets = self.typed_offsets::<T>()?;
+ if let Some((first, last, values_str)) = utf8_span(offsets,
values_buffer) {
self.validate_each_offset::<T, _>(values_buffer.len(),
|string_index, range| {
- if !values_str.is_char_boundary(range.start)
- || !values_str.is_char_boundary(range.end)
- {
+ // `values_str` ends at the last offset. Checking each pair of
offsets doesn't
+ // show that `range.end <= last`: a later offset can still be
smaller, which
+ // `validate_each_offset` reports when it gets there.
+ if range.end > last {
+ return Ok(());
+ }
Review Comment:
This strikes me as awkward; probably how we do validation in general here
could be both refactored and optimized. But I'll defer that to a subsequent PR.
--
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]