HaoYang670 commented on code in PR #1763:
URL: https://github.com/apache/arrow-rs/pull/1763#discussion_r885193911
##########
arrow/src/array/transform/mod.rs:
##########
@@ -394,28 +364,30 @@ impl<'a> MutableArrayData<'a> {
/// a [Capacities] variant is not yet supported.
pub fn with_capacities(
arrays: Vec<&'a ArrayData>,
- mut use_nulls: bool,
+ use_nulls: bool,
capacities: Capacities,
) -> Self {
let data_type = arrays[0].data_type();
use crate::datatypes::*;
// if any of the arrays has nulls, insertions from any array requires
setting bits
// as there is at least one array with nulls.
- if arrays.iter().any(|array| array.null_count() > 0) {
- use_nulls = true;
- };
+ let use_nulls = use_nulls | arrays.iter().any(|array|
array.null_count() > 0);
Review Comment:
> I think this is incorrect, as described above use_nulls is a hint, and
should be false if the only source of nulls are the arras themselves.
Thank you @tustvold for your review. I am still a little confused about it.
This is the truth table in my mind:
| the input value of `use_nulls` | `arrays.any(null_count > 0)` | final
value of `use_nulls`|
| -- | -- | -- |
| true | true | true |
| true | false | true |
| false | true | true |
| false | false | false |
Although I not very clear about the whole logic of the function
`with_capacities`, for `use_nulls` I think this is the logic.
--
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
