alamb commented on code in PR #22158:
URL: https://github.com/apache/datafusion/pull/22158#discussion_r3241311993
##########
datafusion/common/src/utils/mod.rs:
##########
Review Comment:
Any chance we can add comments to this function that explain what it is
doing?
##########
datafusion/common/src/utils/mod.rs:
##########
@@ -1142,17 +1141,18 @@ fn truncate_list_nulls<O: OffsetSizeTrait>(
&Int64Array::new_scalar(0)
};
- let not_empty = neq(&lengths, zero)?;
- let null_and_non_empty = &!nulls.inner() & not_empty.values();
+ let empty = arrow::compute::kernels::cmp::eq(&lengths, zero)?;
+ let valid_or_empty = empty.values() | nulls.inner();
Review Comment:
You might be able to reuse the buffer and save an allocation
```rust
let (mut valid_or_empty, _nulls) =
arrow::compute::kernels::cmp::eq(&lengths, zero)?.into_parts()
valid_or_empty |= nulls.inner();
let valid_or_empty = BooleanArray::from(valid_or_empty);
```
##########
datafusion/common/src/utils/mod.rs:
##########
@@ -1142,17 +1141,18 @@ fn truncate_list_nulls<O: OffsetSizeTrait>(
&Int64Array::new_scalar(0)
};
- let not_empty = neq(&lengths, zero)?;
- let null_and_non_empty = &!nulls.inner() & not_empty.values();
+ let empty = arrow::compute::kernels::cmp::eq(&lengths, zero)?;
Review Comment:
```suggestion
let empty = eq(&lengths, zero)?;
```
Minor style nit is that the other kernels in this code are implementing
using `use arrow::compute::kernels::cmp::eq`
##########
datafusion/common/src/utils/mod.rs:
##########
@@ -1142,17 +1141,18 @@ fn truncate_list_nulls<O: OffsetSizeTrait>(
&Int64Array::new_scalar(0)
};
- let not_empty = neq(&lengths, zero)?;
- let null_and_non_empty = &!nulls.inner() & not_empty.values();
+ let empty = arrow::compute::kernels::cmp::eq(&lengths, zero)?;
+ let valid_or_empty = empty.values() | nulls.inner();
+ let valid_or_empty = BooleanArray::from(valid_or_empty);
Review Comment:
Is the proposal to move the code from `BooleanArray::has_false` to
`BooleanBuffer::has_false` upstream? That seems reasonable for what it is worth
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
