jhorstmann commented on a change in pull request #509:
URL: https://github.com/apache/arrow-rs/pull/509#discussion_r661206212
##########
File path: arrow/src/compute/kernels/take.rs
##########
@@ -516,7 +522,7 @@ where
nulls = match indices.data_ref().null_buffer() {
Some(buffer) => Some(buffer_bin_and(
buffer,
- 0,
+ indices.offset(),
&null_buf.into(),
0,
Review comment:
Yes, null_buf is newly constructed and initialized starting from 0,
while the first buffer and offset pair are coming from the indices array which
might have a non-0 offset.
There was a proposal before to push the offsets down into all buffers
instead of storing it in the array. That way we wouldn't need to care about
which array a buffer originally belonged too. But if we do that we'd still need
a better abstraction for the validity bitmap and only access it via (chunked)
iterators. I'm also not sure whether such a change would have an affect on FFI
usage.
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
