jorgecarleitao commented on a change in pull request #8900:
URL: https://github.com/apache/arrow/pull/8900#discussion_r541774429
##########
File path: rust/arrow/src/compute/kernels/comparison.rs
##########
@@ -47,9 +47,18 @@ macro_rules! compare_op {
let null_bit_buffer =
combine_option_bitmap($left.data_ref(), $right.data_ref(),
$left.len())?;
- let mut result = BooleanBufferBuilder::new($left.len());
+ let byte_capacity = bit_util::ceil($left.len(), 8);
+ let actual_capacity =
bit_util::round_upto_multiple_of_64(byte_capacity);
+ let mut buffer = MutableBuffer::new(actual_capacity);
+ buffer.resize(byte_capacity);
+ let data = buffer.raw_data_mut();
+
for i in 0..$left.len() {
- result.append($op($left.value(i), $right.value(i)))?;
+ if $op($left.value(i), $right.value(i)) {
+ unsafe {
Review comment:
I agree, @Dandandan . Note that all primitives and strings have
iterators and `FromIterator`:
https://github.com/apache/arrow/blob/master/rust/arrow/src/array/iterator.rs ,
but they are for `Option<T>`, not `T`.
I agree with you that we should mark that `fn value` as `unsafe` and offer
an iterator over `T` (besides the one over `Option<T>`). That UB is really
obvious and it is also a security vulnerability causing an escalation of
privileges as it allows privileged access to the application's memory via out
of bounds accesses.
I usually see the iterator over `T` when they can mask the result or OR /
AND the null bitmaps, while `Option<T>` is used when that is not possible /
useful.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
