Jefffrey commented on code in PR #9036:
URL: https://github.com/apache/arrow-rs/pull/9036#discussion_r2678580498
##########
arrow-buffer/src/buffer/run.rs:
##########
@@ -189,6 +189,16 @@ where
&self.run_ends
}
+ /// Similar to [`values`] but accounts for logical slicing, returning only
the values
+ /// that are part of the logical slice of this buffer.
+ ///
+ /// [`values`]: Self::values
+ pub fn values_slice(&self) -> &[E] {
Review Comment:
I quite like the `logical_run_ends()` API actually; it makes it harder to
unintentionally misuse `values_slice()`. I'm thinking we remove
`values_slice()` and expose only `logical_run_ends()`, since users can always
derive `values_slice()` for themselves manually anyway.
My only comment is perhaps rename `logical_run_ends()` to something else,
since it might be confusing with how we describe physical vs logical in the
docstring for the struct. Perhaps something like `sliced_values()`?
##########
arrow-select/src/concat.rs:
##########
@@ -377,20 +381,30 @@ where
PrimitiveArray::<R>::from_iter_values(run_arrays.iter().enumerate().flat_map(
move |(i, run_array)| {
let adjustment = needed_run_end_adjustments[i];
+ let offset =
R::Native::from_usize(run_array.offset()).unwrap();
+ let length = R::Native::from_usize(run_array.len()).unwrap();
+ let start = run_array.get_start_physical_index();
+ let len = run_array.get_end_physical_index() - start + 1;
+
run_array
.run_ends()
.values()
.iter()
- .map(move |run_end| *run_end + adjustment)
+ .skip(start)
+ .take(len)
+ .map(move |run_end| {
+ let value = *run_end - offset;
+ value.min(length) + adjustment
Review Comment:
I'm guessing if we use the new `logical_run_ends()` API we can omit the need
for this offset/min length
--
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]
