neilconway commented on code in PR #11323:
URL: https://github.com/apache/arrow-rs/pull/11323#discussion_r4166668665
##########
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 {
Review Comment:
No, we should be okay -- the issue is that `validate_each_offset` validates
prior offsets, but here we need to check the "next" / "last" offset. As I
mentioned above, I think this whole approach merits refactoring, but I'll do
that in a separate 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]
