This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new e97b1aa7fa fix(arrow-row): return an error instead of wrapping view
offsets past 4 GiB when decoding BinaryView/Utf8View (#11224)
e97b1aa7fa is described below
commit e97b1aa7fa992b544ef7a5a0c98cbc3791a89776
Author: Kita Renji <[email protected]>
AuthorDate: Sun Oct 4 09:29:12 2026 +0000
fix(arrow-row): return an error instead of wrapping view offsets past 4 GiB
when decoding BinaryView/Utf8View (#11224)
## Which issue does this PR close?
- Closes #11011.
## Rationale for this change
`decode_binary_view_inner` casts the data buffer offset to `u32` with no
bound, so once the non-inlined values in a batch go over 4 GiB the views
silently point at the wrong bytes.
The issue left open whether to split the buffer or fail. This PR fails,
since it is the smaller change and keeps the hot loop as is. Splitting
into multiple buffers can come later if people hit this in practice, and
the signatures would not need to change again.
## What changes are included in this PR?
- `decode_binary_view` and `decode_string_view` now return `Result`;
`decode_column` already returns `Result`, so this is internal only.
- New `check_view_buffer_len` helper, called once after the capacity
pass on the summed length of non-inlined values, returns
`InvalidArgumentError` above `u32::MAX`.
- The per-row cast keeps `as u32` with a `debug_assert!`, same approach
as `concat_elements_view_array`.
## Are these changes tested?
Yes, a unit test on the helper with synthetic lengths (`u32::MAX`
passes, `u32::MAX + 1` and `usize::MAX` fail), since actually decoding
more than 4 GiB is too much for a test. Existing arrow-row tests pass.
I ran the `row_format` string view `convert_rows` benches before and
after, but my machine was shared and an A/A run swung by up to 2x, so I
can't claim anything from the numbers beyond no obvious regression. The
release-mode per-row loop is unchanged. The only added work is one
compare per column.
## Are there any user-facing changes?
`RowConverter::convert_rows` now returns an error for this case instead
of returning wrong data. No public API changes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Panchawat <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
Co-authored-by: Jeffrey Vo <[email protected]>
---
arrow-row/src/lib.rs | 4 ++--
arrow-row/src/variable.rs | 34 ++++++++++++++++++++++++++--------
2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/arrow-row/src/lib.rs b/arrow-row/src/lib.rs
index 7ffb54a6f9..fa0d710e6c 100644
--- a/arrow-row/src/lib.rs
+++ b/arrow-row/src/lib.rs
@@ -2293,11 +2293,11 @@ unsafe fn decode_column(
DataType::Boolean => Arc::new(decode_bool(rows, options)),
DataType::Binary => Arc::new(decode_binary::<i32>(rows,
options)),
DataType::LargeBinary => Arc::new(decode_binary::<i64>(rows,
options)),
- DataType::BinaryView => Arc::new(decode_binary_view(rows,
options)),
+ DataType::BinaryView => Arc::new(decode_binary_view(rows,
options)?),
DataType::FixedSizeBinary(size) =>
Arc::new(decode_fixed_size_binary(rows, size, options)),
DataType::Utf8 => Arc::new(unsafe{ decode_string::<i32>(rows,
options, validate_utf8) }),
DataType::LargeUtf8 => Arc::new(unsafe {
decode_string::<i64>(rows, options, validate_utf8) }),
- DataType::Utf8View => Arc::new(unsafe {
decode_string_view(rows, options, validate_utf8) }),
+ DataType::Utf8View => Arc::new(unsafe {
decode_string_view(rows, options, validate_utf8) }?),
_ => return
Err(ArrowError::NotYetImplemented(format!("unsupported data type: {data_type}"
)))
}
}
diff --git a/arrow-row/src/variable.rs b/arrow-row/src/variable.rs
index 2ae132c48d..814d9c3b5d 100644
--- a/arrow-row/src/variable.rs
+++ b/arrow-row/src/variable.rs
@@ -23,7 +23,7 @@ use arrow_buffer::{
ArrowNativeType, BooleanBuffer, MutableBuffer, NullBuffer, OffsetBuffer,
ScalarBuffer,
};
use arrow_data::MAX_INLINE_VIEW_LEN;
-use arrow_schema::SortOptions;
+use arrow_schema::{ArrowError, SortOptions};
use builder::make_view;
/// The block size of the variable length encoding
@@ -310,7 +310,7 @@ pub fn decode_binary<I: OffsetSizeTrait>(
fn decode_binary_view_inner<const VALIDATE_UTF8: bool>(
rows: &mut [&[u8]],
options: SortOptions,
-) -> BinaryViewArray {
+) -> Result<BinaryViewArray, ArrowError> {
let len = rows.len();
let inline_str_max_len = MAX_INLINE_VIEW_LEN as usize;
@@ -327,6 +327,15 @@ fn decode_binary_view_inner<const VALIDATE_UTF8: bool>(
inline_capacity += len;
}
}
+ // Every view offset is at most the total length of the non-inlined values,
+ // so checking it once here keeps the offsets below within the `i32` range
+ // the Arrow spec requires
+ let long_values_len = values_capacity - inline_str_max_len;
+ if long_values_len > i32::MAX as usize {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "{long_values_len} bytes of non-inlined values too long to decode
into a view array with a single data buffer"
+ )));
+ }
let mut values = MutableBuffer::new(values_capacity);
let mut view_utf8_validation_buffer = if VALIDATE_UTF8 {
Vec::with_capacity(inline_capacity)
@@ -373,11 +382,17 @@ fn decode_binary_view_inner<const VALIDATE_UTF8: bool>(
// SAFETY:
// Valid by construction above
- unsafe { BinaryViewArray::new_unchecked(views.into(),
[values.into()].into(), nulls) }
+ Ok(unsafe { BinaryViewArray::new_unchecked(views.into(),
[values.into()].into(), nulls) })
}
/// Decodes a binary view array from `rows` with the provided `options`
-pub fn decode_binary_view(rows: &mut [&[u8]], options: SortOptions) ->
BinaryViewArray {
+///
+/// Returns an error if the non-inlined values are longer than `i32::MAX`
bytes,
+/// the most a single data buffer can address
+pub fn decode_binary_view(
+ rows: &mut [&[u8]],
+ options: SortOptions,
+) -> Result<BinaryViewArray, ArrowError> {
decode_binary_view_inner::<false>(rows, options)
}
@@ -409,17 +424,20 @@ pub unsafe fn decode_string<I: OffsetSizeTrait>(
/// # Safety
///
/// The row must contain valid UTF-8 data
+///
+/// Returns an error if the non-inlined values are longer than `i32::MAX`
bytes,
+/// the most a single data buffer can address
pub unsafe fn decode_string_view(
rows: &mut [&[u8]],
options: SortOptions,
validate_utf8: bool,
-) -> StringViewArray {
+) -> Result<StringViewArray, ArrowError> {
let view = if validate_utf8 {
- decode_binary_view_inner::<true>(rows, options)
+ decode_binary_view_inner::<true>(rows, options)?
} else {
- decode_binary_view_inner::<false>(rows, options)
+ decode_binary_view_inner::<false>(rows, options)?
};
- unsafe { view.to_string_view_unchecked() }
+ Ok(unsafe { view.to_string_view_unchecked() })
}
pub fn decode_null_value(rows: &mut [&[u8]], options: SortOptions) {
