From 4b2da1ab35a7b7bd3d40a325a1f13d8b0b77a20d Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Mon, 6 Jul 2026 15:48:47 +0900 Subject: [PATCH 1/3] Compact run buffer slice values --- arrow-buffer/src/buffer/run.rs | 68 +++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/arrow-buffer/src/buffer/run.rs b/arrow-buffer/src/buffer/run.rs index 703ae913801d..9c6885ae95b8 100644 --- a/arrow-buffer/src/buffer/run.rs +++ b/arrow-buffer/src/buffer/run.rs @@ -277,9 +277,32 @@ where logical_offset.saturating_add(logical_length) <= self.logical_length, "the length + offset of the sliced RunEndBuffer cannot exceed the existing length" ); - Self { + let logical_offset = self.logical_offset + logical_offset; + + // tmp makes it easier to use the get physical index methods here + let tmp = Self { run_ends: self.run_ends.clone(), - logical_offset: self.logical_offset + logical_offset, + logical_offset, + logical_length, + }; + let start = tmp.get_start_physical_index(); + let end = tmp.get_end_physical_index(); + + // if we're slicing past some of the physical runs at the start, we can + // omit them entirely as long as we adjust offset accordingly + let (physical_start, logical_offset) = if start > 0 { + let new_offset = logical_offset - self.run_ends[start - 1].as_usize(); + (start, new_offset) + } else { + (0, logical_offset) + }; + + Self { + run_ends: self + .run_ends + // its easier to omit the physical end values which may be unreferenced + .slice(physical_start, end - physical_start + 1), + logical_offset, logical_length, } } @@ -421,4 +444,45 @@ mod tests { let sliced_values: Vec = sliced.sliced_values().collect(); assert_eq!(sliced_values, &[2]); } + + #[test] + fn test_compact_slicing() { + // {} represents a logical slice within physically expanded [] + // [{A, A, A, A, B, B, B, B, C, C, C, C}] + let buffer = RunEndBuffer::::new(vec![4, 8, 12].into(), 0, 12); + + // zero slice + let slice = buffer.slice(0, 0); + assert_eq!(slice.len(), 0); + assert_eq!(slice.offset(), 0); + assert_eq!(slice.values(), &[4]); + + // compact start only + // [B, B, {B, B, C, C, C, C}] + let slice = buffer.slice(6, 6); + assert_eq!(slice.len(), 6); + assert_eq!(slice.offset(), 2); // since we sliced away the 4 run + assert_eq!(slice.values(), &[8, 12]); + + // compact end only + // [A, {A, A, A, B, B, B}, B] + let slice = buffer.slice(1, 6); + assert_eq!(slice.len(), 6); + assert_eq!(slice.offset(), 1); + assert_eq!(slice.values(), &[4, 8]); + + // compact both + // [B, {B, B}, B, B, B, B, B] + let slice = buffer.slice(5, 2); + assert_eq!(slice.len(), 2); + assert_eq!(slice.offset(), 1); + assert_eq!(slice.values(), &[8]); + + // no compaction + // [A, {A, A, A, B, B, B, B, C, C, C}, C] + let slice = buffer.slice(1, 10); + assert_eq!(slice.len(), 10); + assert_eq!(slice.offset(), 1); + assert_eq!(slice.values(), &[4, 8, 12]); + } } From e504e68fbe80a7b82ebd5861c8cc7b900b3de778 Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Mon, 6 Jul 2026 23:17:33 +0900 Subject: [PATCH 2/3] dont need to adjust offset actually --- arrow-buffer/src/buffer/run.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/arrow-buffer/src/buffer/run.rs b/arrow-buffer/src/buffer/run.rs index 9c6885ae95b8..fe896b1faff9 100644 --- a/arrow-buffer/src/buffer/run.rs +++ b/arrow-buffer/src/buffer/run.rs @@ -288,20 +288,8 @@ where let start = tmp.get_start_physical_index(); let end = tmp.get_end_physical_index(); - // if we're slicing past some of the physical runs at the start, we can - // omit them entirely as long as we adjust offset accordingly - let (physical_start, logical_offset) = if start > 0 { - let new_offset = logical_offset - self.run_ends[start - 1].as_usize(); - (start, new_offset) - } else { - (0, logical_offset) - }; - Self { - run_ends: self - .run_ends - // its easier to omit the physical end values which may be unreferenced - .slice(physical_start, end - physical_start + 1), + run_ends: self.run_ends.slice(start, end - start + 1), logical_offset, logical_length, } @@ -458,11 +446,13 @@ mod tests { assert_eq!(slice.values(), &[4]); // compact start only - // [B, B, {B, B, C, C, C, C}] + // [B, B, B, B, B, B, {B, B, C, C, C, C}] let slice = buffer.slice(6, 6); assert_eq!(slice.len(), 6); - assert_eq!(slice.offset(), 2); // since we sliced away the 4 run + assert_eq!(slice.offset(), 6); assert_eq!(slice.values(), &[8, 12]); + let values = slice.sliced_values().collect::>(); + assert_eq!(values, &[2, 6]); // compact end only // [A, {A, A, A, B, B, B}, B] @@ -470,13 +460,17 @@ mod tests { assert_eq!(slice.len(), 6); assert_eq!(slice.offset(), 1); assert_eq!(slice.values(), &[4, 8]); + let values = slice.sliced_values().collect::>(); + assert_eq!(values, &[3, 6]); // compact both - // [B, {B, B}, B, B, B, B, B] + // [B, B, B, B, B, {B, B}, B] let slice = buffer.slice(5, 2); assert_eq!(slice.len(), 2); - assert_eq!(slice.offset(), 1); + assert_eq!(slice.offset(), 5); assert_eq!(slice.values(), &[8]); + let values = slice.sliced_values().collect::>(); + assert_eq!(values, &[2]); // no compaction // [A, {A, A, A, B, B, B, B, C, C, C}, C] @@ -484,5 +478,7 @@ mod tests { assert_eq!(slice.len(), 10); assert_eq!(slice.offset(), 1); assert_eq!(slice.values(), &[4, 8, 12]); + let values = slice.sliced_values().collect::>(); + assert_eq!(values, &[3, 7, 10]); } } From 5575583d260610c081b42f7ddeb79d607bf7fdb7 Mon Sep 17 00:00:00 2001 From: Jefffrey Date: Thu, 23 Jul 2026 21:53:23 +0900 Subject: [PATCH 3/3] apply slicing optimization to runarray too, fix tests --- arrow-array/src/array/run_array.rs | 102 +++++++++++++++++++---------- arrow-buffer/src/buffer/run.rs | 12 +++- 2 files changed, 77 insertions(+), 37 deletions(-) diff --git a/arrow-array/src/array/run_array.rs b/arrow-array/src/array/run_array.rs index 9a56b01fcadf..19a131e8be12 100644 --- a/arrow-array/src/array/run_array.rs +++ b/arrow-array/src/array/run_array.rs @@ -345,10 +345,50 @@ impl RunArray { /// /// - Specified slice (`offset` + `length`) exceeds existing length pub fn slice(&self, offset: usize, length: usize) -> Self { + assert!( + offset.saturating_add(length) <= self.run_ends.len(), + "the length + offset of the sliced array cannot exceed the existing length" + ); + + if length == 0 { + return Self { + data_type: self.data_type.clone(), + run_ends: self.run_ends.slice(0, 0), + values: self.values.slice(0, 0), + }; + } + + // Compact the physical run ends & values to only what is strictly + // needed. + + // tmp makes it easier to use the get physical index methods here + // SAFETY: new offset + slice is within bounds, and existing values are + // valid + let tmp = unsafe { + RunEndBuffer::new_unchecked( + self.run_ends.inner().clone(), + self.run_ends.offset() + offset, + length, + ) + }; + let start = tmp.get_start_physical_index(); + let end = tmp.get_end_physical_index(); + + let values = self.values.slice(start, end - start + 1); + // SAFETY: existing values are valid, and values referenced by the logical + // offset are preserved + let run_ends = unsafe { + RunEndBuffer::new_unchecked( + tmp.inner().slice(start, end - start + 1), + tmp.offset(), + tmp.len(), + ) + }; + Self { data_type: self.data_type.clone(), - run_ends: self.run_ends.slice(offset, length), - values: self.values.clone(), + run_ends, + values, } } } @@ -894,6 +934,7 @@ mod tests { }; }); } + #[test] fn test_run_array() { // Construct a value array @@ -1220,7 +1261,6 @@ mod tests { PrimitiveRunBuilder::::with_capacity(input_array.len()); builder.extend(input_array.iter().copied()); let run_array = builder.finish(); - let physical_values_array = run_array.values().as_primitive::(); // test for all slice lengths. for slice_len in 1..=total_len { @@ -1234,15 +1274,11 @@ mod tests { // test for offset = 0 and slice length = slice_len // slice the input array using which the run array was built. let sliced_input_array = &input_array[0..slice_len]; - - // slice the run array - let sliced_run_array: RunArray = - run_array.slice(0, slice_len).into_data().into(); - - // Get physical indices. + let sliced_run_array = run_array.slice(0, slice_len); let physical_indices = sliced_run_array .get_physical_indices(&logical_indices) .unwrap(); + let physical_values_array = sliced_run_array.values().as_primitive::(); compare_logical_and_physical_indices( &logical_indices, @@ -1254,17 +1290,11 @@ mod tests { // test for offset = total_len - slice_len and slice length = slice_len // slice the input array using which the run array was built. let sliced_input_array = &input_array[total_len - slice_len..total_len]; - - // slice the run array - let sliced_run_array: RunArray = run_array - .slice(total_len - slice_len, slice_len) - .into_data() - .into(); - - // Get physical indices + let sliced_run_array = run_array.slice(total_len - slice_len, slice_len); let physical_indices = sliced_run_array .get_physical_indices(&logical_indices) .unwrap(); + let physical_values_array = sliced_run_array.values().as_primitive::(); compare_logical_and_physical_indices( &logical_indices, @@ -1291,8 +1321,11 @@ mod tests { let slices = [(0, 12), (0, 2), (2, 5), (3, 0), (3, 3), (3, 4), (4, 8)]; for (offset, length) in slices { let a = array.slice(offset, length); - let n = a.logical_nulls().unwrap(); - let n = n.into_iter().collect::>(); + let n = if let Some(nulls) = a.logical_nulls() { + nulls.into_iter().collect::>() + } else { + vec![true; length] + }; assert_eq!(&n, &expected[offset..offset + length], "{offset} {length}"); } } @@ -1364,33 +1397,32 @@ mod tests { #[test] fn test_run_array_values_slice() { - // 0, 0, 1, 1, 1, 2...2 (15 2s) - let run_ends: PrimitiveArray = vec![2, 5, 20].into(); + // 0, 0, 1, 1, 1, 2 + let run_ends: PrimitiveArray = vec![2, 5, 6].into(); let values: PrimitiveArray = vec![0, 1, 2].into(); let array = RunArray::::try_new(&run_ends, &values).unwrap(); - let slice = array.slice(1, 4); // 0 | 1, 1, 1 | + let slice = array.slice(1, 4); // 0, 1, 1, 1 | // logical indices: 1, 2, 3, 4 // physical indices: 0, 1, 1, 1 - // values at 0 is 0 - // values at 1 is 1 - // values slice should be [0, 1] assert_eq!(slice.get_start_physical_index(), 0); assert_eq!(slice.get_end_physical_index(), 1); - let values_slice = slice.values_slice(); - let values_slice = values_slice.as_primitive::(); - assert_eq!(values_slice.values(), &[0, 1]); + assert_eq!(slice.run_ends.len(), 4); + assert_eq!(slice.run_ends.offset(), 1); + assert_eq!(slice.run_ends.values(), &[2, 5]); + assert_eq!(slice.values().as_ref(), &Int32Array::from(vec![0, 1])); let slice2 = array.slice(2, 3); // 1, 1, 1 // logical indices: 2, 3, 4 - // physical indices: 1, 1, 1 - assert_eq!(slice2.get_start_physical_index(), 1); - assert_eq!(slice2.get_end_physical_index(), 1); - - let values_slice2 = slice2.values_slice(); - let values_slice2 = values_slice2.as_primitive::(); - assert_eq!(values_slice2.values(), &[1]); + // physical indices: 0, 0, 0 + assert_eq!(slice2.get_start_physical_index(), 0); + assert_eq!(slice2.get_end_physical_index(), 0); + + assert_eq!(slice2.run_ends.len(), 3); + assert_eq!(slice2.run_ends.offset(), 2); + assert_eq!(slice2.run_ends.values(), &[5]); + assert_eq!(slice2.values().as_ref(), &Int32Array::from(vec![1])); } #[test] diff --git a/arrow-buffer/src/buffer/run.rs b/arrow-buffer/src/buffer/run.rs index fe896b1faff9..f3606663d116 100644 --- a/arrow-buffer/src/buffer/run.rs +++ b/arrow-buffer/src/buffer/run.rs @@ -279,6 +279,14 @@ where ); let logical_offset = self.logical_offset + logical_offset; + if logical_length == 0 { + return Self { + run_ends: self.run_ends.slice(0, 0), + logical_length: 0, + logical_offset: 0, + }; + } + // tmp makes it easier to use the get physical index methods here let tmp = Self { run_ends: self.run_ends.clone(), @@ -440,10 +448,10 @@ mod tests { let buffer = RunEndBuffer::::new(vec![4, 8, 12].into(), 0, 12); // zero slice - let slice = buffer.slice(0, 0); + let slice = buffer.slice(2, 0); assert_eq!(slice.len(), 0); assert_eq!(slice.offset(), 0); - assert_eq!(slice.values(), &[4]); + assert!(slice.values().is_empty()); // compact start only // [B, B, B, B, B, B, {B, B, C, C, C, C}]