You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/10/19 13:56:50 UTC
[I] Add `RecordBatch::take` [arrow-rs]
alamb opened a new issue, #4958:
URL: https://github.com/apache/arrow-rs/issues/4958
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
When trying to divide up data in `RecordBatches` into subsets such as when writing partitioned output in DataFusion or in lancedb, we end up creating a new `RecordBatch` that is a subset of the input eventually using the underlying arrow [take](https://docs.rs/arrow-select/latest/arrow_select/take/fn.take.html) kernel
See slack discussion in https://the-asf.slack.com/archives/C01QUFS30TD/p1696014226063749
**Describe the solution you'd like**
Similarly to [`RecordBatch::slice`](https://docs.rs/arrow/latest/arrow/array/struct.RecordBatch.html#method.slice) I would like an ergnomic version of [`RecordBatch::take`] that can be used like
```rust
let batch: RecordBatch = ...;
let indices = vec![1,3,5];
// new_batch contains rows 1, 3, and 5 of `batch`
let new_batch = batch.take(&indices)?;
```
We can probably port the code from DataFuson here (thanks @devinjdangelo for the link)
https://github.com/apache/arrow-datafusion/blob/85f3578f5fb47d28a8bc3a7b9be0284b3ced0fcd/datafusion/physical-plan/src/repartition/mod.rs#L193-L212
**Describe alternatives you've considered**
If we worry about RecordBatch getting to complicated, we might want to consider adding `RecordBatchExt` trait that contains these methods, but that might be overly complicated
**Additional context**
cc @devinjdangelo @westonpace
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Add `RecordBatch::take` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4958:
URL: https://github.com/apache/arrow-rs/issues/4958#issuecomment-1771151203
Making a bunch of traits in different crates certainly doesn't feel like it would make this functionality easier to discover. Perhaps the best thing is to use free functions and make sure the documentation clearly points people at them
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Add `RecordBatch::take` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4958:
URL: https://github.com/apache/arrow-rs/issues/4958#issuecomment-1771046185
Given this is well specified and there is existing code, I think this would be a great project for new members of the community.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Add `RecordBatch::take` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #4958:
URL: https://github.com/apache/arrow-rs/issues/4958#issuecomment-1771146312
> The upside of RecordBatch::take is that it is easier to discover than a free function in another crate.
It would need to be a trait implemented in arrow-select to not run into the orphan rule
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Add `RecordBatch::take` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4958:
URL: https://github.com/apache/arrow-rs/issues/4958#issuecomment-1771144191
The upside of `RecordBatch::take` is that it is easier to discover than a free function in another crate.
For this reason, an extension trait (perhaps in the `arrow` crate) along with links from `RecordBatch` documentation would be preferable in my mind.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Add take_record_batch kernel [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #4958: Add take_record_batch kernel
URL: https://github.com/apache/arrow-rs/issues/4958
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Add `RecordBatch::take` [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #4958:
URL: https://github.com/apache/arrow-rs/issues/4958#issuecomment-1771113767
RecordBatch is defined in arrow-array, whereas the take kernel is defined in arrow-select.
One possibility would be an extension trait to provide this method, but I think a simpler option would be to simply provide something like https://docs.rs/arrow-select/latest/arrow_select/filter/fn.filter_record_batch.html but for the take kernel
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Add take_record_batch kernel [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #4958:
URL: https://github.com/apache/arrow-rs/issues/4958#issuecomment-1912347795
FWIW this came up again in IOx internally
BTW the idea in https://github.com/lancedb/lance/pull/1337 is pretty neat (add a `RecordBatchExt` extension trait for RecordBatch -- which perhaps could be a way to work aroudn this - put the `RecordBatchExtc` in the main arrow crate rather than the `record_batch` crate) 🤔
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org