alamb commented on a change in pull request #509:
URL: https://github.com/apache/arrow-rs/pull/509#discussion_r660960513
##########
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:
Is it correct that this `0` is due to the fact that `null_buf` was
constructed via
```rust
let mut null_buf =
MutableBuffer::new(num_byte).with_bitset(num_byte, true);
```
in the same `else` clause?
##########
File path: arrow/src/compute/kernels/take.rs
##########
@@ -876,6 +900,48 @@ mod tests {
.unwrap();
}
+ #[test]
+ fn test_take_primitive_nullable_indices_non_null_values_with_offset() {
+ let index =
+ UInt32Array::from(vec![Some(0), Some(1), Some(2), Some(3), None,
None]);
+ let index = index.slice(2, 4);
+ let index = index.as_any().downcast_ref::<UInt32Array>().unwrap();
+
+ assert_eq!(
+ index,
+ &UInt32Array::from(vec![Some(2), Some(3), None, None])
+ );
+
+ test_take_primitive_arrays_non_null::<Int64Type>(
+ vec![0, 1, 2, 3, 4, 5, 6],
Review comment:
Would it make sense to use different values here than the indices --
perhaps something like
```suggestion
vec![0, 10, 20, 30, 40, 50, 60],
```
So it is clearer from just this context that just `20` and `30` should be
returned
--
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
