You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by gi...@apache.org on 2026/10/19 20:46:19 UTC
(arrow-rs) 01/01: fix: use correct length in runarray validation (#11668)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11668-837b6703f0ea8e33b891584511bb99d84794d2ef
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit a9455d4a269d1edce4891c0927fed1c2bf24fb72
Author: Jeffrey Vo <je...@gmail.com>
AuthorDate: Mon Oct 19 18:07:00 2026 +0000
fix: use correct length in runarray validation (#11668)
# Which issue does this PR close?
<!--
It is not necessary to raise an issue, but it is encouraged if it is a
non-trivial
enhancement/bug.
If this PR relates to an existing issue, but does not fully resolve it,
please change
the wording below from "Closes #NNN" to "Relates to #NNN" or "Part of
#NNN" to avoid
false closing an issue.
-->
- Closes #11587
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly
in the
issue then this section is not needed. Explaining clearly why changes
are proposed
helps reviewers understand your changes and offer better suggestions for
fixes.
Please try to explain the problem you are trying to solve in terms of
the user-visible
behavior, rather than the implementation.
-->
Fix incorrect validation of run arrays
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it
is
sometimes worth providing a summary of the individual changes in this
PR.
-->
Use correct parent logical length when validating run values
# Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they
covered by existing tests, if so identify these tests)?
If this PR claims a performance improvement, please include evidence
such as
benchmark results.
DO NOT simply paste a list of commands ran to verify the code; this is
what our
CI is for and it pollutes the PR. ONLY explain if testing was included
with this
PR, or why it wasn't.
-->
Yes
# Are there any significant user-facing changes?
<!--
Significant user-facing changes include:
- Breaking public API change
- Significant change to existing functionality (e.g. function returns
something different)
These need to be explicitly called out to ensure we have proper
documentation if
required and to ensure we follow proper semantic versioning.
DO NOT restate the changes of this PR; if a bug was fixed and already
explained
above, we don't need this to be restated as a user-facing change.
-->
No
---------
Co-authored-by: Andrew Lamb <an...@nerdnetworks.org>
---
arrow-data/src/data.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 111 insertions(+), 7 deletions(-)
diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs
index 8fea7f327d..f1f1621fab 100644
--- a/arrow-data/src/data.rs
+++ b/arrow-data/src/data.rs
@@ -1675,11 +1675,19 @@ impl ArrayData {
}
}
DataType::RunEndEncoded(run_ends, _values) => {
+ let len_plus_offset =
+ checked_len_plus_offset(&self.data_type, self.len, self.offset)?;
let run_ends_data = self.child_at(0)?;
match run_ends.data_type() {
- DataType::Int16 => run_ends_data.check_run_ends::<i16>(),
- DataType::Int32 => run_ends_data.check_run_ends::<i32>(),
- DataType::Int64 => run_ends_data.check_run_ends::<i64>(),
+ DataType::Int16 => {
+ run_ends_data.check_run_ends::<i16>(self.len, len_plus_offset)
+ }
+ DataType::Int32 => {
+ run_ends_data.check_run_ends::<i32>(self.len, len_plus_offset)
+ }
+ DataType::Int64 => {
+ run_ends_data.check_run_ends::<i64>(self.len, len_plus_offset)
+ }
data_type => Err(ArrowError::InvalidArgumentError(format!(
"Run end type must be Int16, Int32 or Int64, got {data_type}"
))),
@@ -1829,7 +1837,11 @@ impl ArrayData {
}
/// Validates that each value in run_ends array is positive and strictly increasing.
- fn check_run_ends<T>(&self) -> Result<(), ArrowError>
+ fn check_run_ends<T>(
+ &self,
+ parent_len: usize,
+ parent_len_plus_offset: usize,
+ ) -> Result<(), ArrowError>
where
T: ArrowNativeType + TryInto<i64> + num_traits::Num + std::fmt::Display,
{
@@ -1856,10 +1868,9 @@ impl ArrayData {
Ok(())
})?;
- let len_plus_offset = checked_len_plus_offset(&self.data_type, self.len, self.offset)?;
- if prev_value.as_usize() < len_plus_offset {
+ if parent_len > 0 && prev_value.as_usize() < parent_len_plus_offset {
return Err(ArrowError::InvalidArgumentError(format!(
- "The offset + length of array should be less or equal to last value in the run_ends array. The last value of run_ends array is {prev_value} and offset + length of array is {len_plus_offset}."
+ "The offset + length of array should be less or equal to last value in the run_ends array. The last value of run_ends array is {prev_value} and offset + length of array is {parent_len_plus_offset}."
)));
}
Ok(())
@@ -3570,6 +3581,99 @@ mod tests {
);
}
+ /// Returns a `RunEndEncoded<Int32, Float32>` data type and run ends / values
+ /// children whose runs end at `[1, 3]`, i.e. a logical length of 3.
+ fn run_array_test_children() -> (DataType, Vec<ArrayData>) {
+ let dt = DataType::RunEndEncoded(
+ Arc::new(Field::new(
+ Field::REE_RUN_ENDS_FIELD_DEFAULT_NAME,
+ DataType::Int32,
+ false,
+ )),
+ Arc::new(Field::new(
+ Field::REE_VALUES_FIELD_DEFAULT_NAME,
+ DataType::Float32,
+ true,
+ )),
+ );
+
+ let runs = ArrayData::builder(DataType::Int32)
+ .len(2)
+ .add_buffer(vec![1_i32, 3].into())
+ .build()
+ .unwrap();
+ let values = ArrayData::builder(DataType::Float32)
+ .len(2)
+ .add_buffer(vec![1.5_f32, 2.5].into())
+ .build()
+ .unwrap();
+
+ (dt, vec![runs, values])
+ }
+
+ #[test]
+ fn test_run_array_validates_last_run() {
+ let (dt, children) = run_array_test_children();
+
+ let err = ArrayDataBuilder::new(dt)
+ .len(4)
+ .offset(0)
+ .child_data(children)
+ .build()
+ .unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: The offset + length of array should be less or equal to last value in the run_ends array. The last value of run_ends array is 3 and offset + length of array is 4."
+ );
+ }
+
+ #[test]
+ fn test_run_array_empty_validates() {
+ let (dt, children) = run_array_test_children();
+
+ let data = ArrayDataBuilder::new(dt)
+ .len(0)
+ .offset(4)
+ .child_data(children)
+ .build()
+ .unwrap();
+ assert_eq!(data.len(), 0);
+ }
+
+ /// The parent's offset must be included when checking the last run end:
+ /// offset 2 + length 2 = 4 exceeds the last run end of 3.
+ #[test]
+ fn test_run_array_validates_last_run_with_offset() {
+ let (dt, children) = run_array_test_children();
+
+ let err = ArrayDataBuilder::new(dt)
+ .len(2)
+ .offset(2)
+ .child_data(children)
+ .build()
+ .unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: The offset + length of array should be less or equal to last value in the run_ends array. The last value of run_ends array is 3 and offset + length of array is 4."
+ );
+ }
+
+ /// A slice that stays within the last run end is valid:
+ /// offset 1 + length 2 = 3 equals the last run end of 3.
+ #[test]
+ fn test_run_array_sliced_validates() {
+ let (dt, children) = run_array_test_children();
+
+ let data = ArrayDataBuilder::new(dt)
+ .len(2)
+ .offset(1)
+ .child_data(children)
+ .build()
+ .unwrap();
+ assert_eq!(data.len(), 2);
+ assert_eq!(data.offset(), 1);
+ }
+
/// `validate_values` must report missing children rather than index out of bounds.
#[test]
#[cfg(not(feature = "force_validate"))]