Rich-T-kid commented on code in PR #11174:
URL: https://github.com/apache/arrow-rs/pull/11174#discussion_r4321676862
##########
arrow-select/src/coalesce.rs:
##########
@@ -809,6 +930,26 @@ trait InProgressArray: std::fmt::Debug + Send + Sync {
}
}
+ /// Copy rows at `indices` from `source` into the in-progress array.
+ /// `indices` must already be bounds-checked against `source`.
+ fn copy_rows_by_indices(
+ &mut self,
+ source: &ArrayRef,
+ indices: &[u32],
+ ) -> Result<(), ArrowError> {
+ if indices.is_empty() {
+ return Ok(());
+ }
+ let buf = Buffer::from_slice_ref(indices);
+ let scalar: ScalarBuffer<u32> = ScalarBuffer::new(buf, 0,
indices.len());
+ let indices_arr = UInt32Array::new(scalar, None);
+ let taken = take(source.as_ref(), &indices_arr, None)?;
+ self.set_source(Some(taken));
+ let result = self.copy_rows(0, indices.len());
+ self.set_source(None);
Review Comment:
yeah this is a good point, Initially this method was called in
`push_batch_with_indices` but I reverted it as there were some performance
issues related to non primitives and view types.
see https://github.com/apache/arrow-rs/issues/11567 for a bit more context.
I think its worth removing the gate and creating follow up issues/tickets
for possible regressions. I have https://github.com/apache/arrow-rs/pull/11732
which will address the boolean regression, we should do similar things for the
other datatypes (separate PR's each).
Ill remove the gate and re-run the benchmarks
--
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]
