Rich-T-kid commented on issue #8879:
URL: https://github.com/apache/arrow-rs/issues/8879#issuecomment-5086350738
> wouldn't the simplest solution be to allow for callers to inject this
TakeOption and in datafusion we can set the check_bounds to false to avoid
un-needed checks.
to avoid making this a breaking API change we could also change the order
slightly
```rust
pub fn take_record_batch(
record_batch: &RecordBatch,
indices: &dyn Array,
) -> Result<RecordBatch, ArrowError> {
let columns = record_batch
.columns()
.iter()
.map(|c| take(c, indices, None))
.collect::<Result<Vec<_>, _>>()?;
RecordBatch::try_new(record_batch.schema(), columns)
}
```
instead of calling take on each array which would do the bounds check for
each call we could
```rust
pub fn take_record_batch(
record_batch: &RecordBatch,
indices: &dyn Array,
) -> Result<RecordBatch, ArrowError> {
let mut columns = record_batch.columns().iter();
let mut taken = Vec::with_capacity(record_batch.num_columns());
if let Some(first) = columns.next() {
// the only call that actually validates. a bad index surfaces
// as an ArrowError here instead of panicking later
taken.push(take(first, indices, options)?);
}
// every column in a RecordBatch has the same length, so if the first
// column's indices were in bounds, so are everyone else's
let unchecked = Some(TakeOptions { check_bounds: false });
for column in columns {
taken.push(take(column, indices, unchecked.clone())?);
}
RecordBatch::try_new(record_batch.schema(), taken)
}
```
Ill run some benchmarks and see if this shows a similar result to what you
mentioned 👍
--
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]
