alamb commented on code in PR #6168:
URL: https://github.com/apache/arrow-rs/pull/6168#discussion_r1700805149
##########
arrow-select/src/take.rs:
##########
@@ -487,11 +487,9 @@ fn take_byte_view<T: ByteViewType, IndexType:
ArrowPrimitiveType>(
) -> Result<GenericByteViewArray<T>, ArrowError> {
let new_views = take_native(array.views(), indices);
let new_nulls = take_nulls(array.nulls(), indices);
- Ok(GenericByteViewArray::new(
- new_views,
- array.data_buffers().to_vec(),
- new_nulls,
- ))
+ Ok(unsafe {
Review Comment:
I think some of our other contributors suggest annotating the code with some
justification of why unsafe is ok
```suggestion
// Safety: array.views was valid, and take_native copies only valid
values, and verifies bounds
Ok(unsafe {
```
##########
arrow/src/util/bench_util.rs:
##########
@@ -160,6 +160,34 @@ pub fn create_string_array_with_len<Offset:
OffsetSizeTrait>(
.collect()
}
+/// Creates a random (but fixed-seeded) string view array of a given size and
null density.
+///
+/// See `create_string_array` above for more details.
+pub fn create_string_view_array(size: usize, null_density: f32) ->
StringViewArray {
+ create_string_view_array_with_max_len(size, null_density, 400)
Review Comment:
Using a max len of 400 means most of the string values will be "long" (not
inlined views which happens for values less than 12 bytes in length). I don't
think that is critical I am just pointing it out
--
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]
