Rich-T-kid commented on code in PR #10292:
URL: https://github.com/apache/arrow-rs/pull/10292#discussion_r3927265934


##########
arrow-array/src/array/run_array.rs:
##########
@@ -345,10 +345,50 @@ impl<R: RunEndIndexType> RunArray<R> {
     ///
     /// - 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(),
+            )
+        };

Review Comment:
   okay makes sense to me



##########
arrow-array/src/array/run_array.rs:
##########
@@ -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<Int16Type> =
-                run_array.slice(0, slice_len).into_data().into();
-
-            // Get physical indices.
+            let sliced_run_array = run_array.slice(0, slice_len);

Review Comment:
   looks alot cleaner



-- 
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]

Reply via email to