Jefffrey commented on code in PR #10810:
URL: https://github.com/apache/arrow-rs/pull/10810#discussion_r3862574479
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -41,12 +44,33 @@ use parquet_variant::{
use std::borrow::Cow;
use std::sync::Arc;
-/// Returns the raw bytes at the given index from a binary-like array, return
`None` if the array isn't binary-like.
+/// Returns the logical bytes at the given index from a binary-like array,
resolving dictionary
+/// and run-end encodings. Returns `None` for nulls or if the logical values
aren't binary-like.
pub(crate) fn binary_array_value(array: &dyn Array, index: usize) ->
Option<&[u8]> {
match array.data_type() {
- DataType::Binary => Some(array.as_binary::<i32>().value(index)),
- DataType::LargeBinary => Some(array.as_binary::<i64>().value(index)),
- DataType::BinaryView => Some(array.as_binary_view().value(index)),
+ DataType::Binary => array
Review Comment:
we can pull the null check to the top of the function to avoid the
duplication in the arms:
```rust
pub(crate) fn binary_array_value(array: &dyn Array, index: usize) ->
Option<&[u8]> {
if array.is_null(index) {
return None;
}
match array.data_type() {
```
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -285,18 +328,14 @@ impl VariantArray {
/// # Requirements of the `StructArray`
///
/// 1. A required field named `metadata` which is binary, large_binary, or
- /// binary_view
+ /// binary_view, optionally dictionary- or run-end-encoded
Review Comment:
```suggestion
/// binary_view, optionally dictionary or run-end-encoded
```
--
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]
