Re: [PR] [Variant] Support typed access for numeric types in variant_get [arrow-rs]

[via GitHub]

superserious-dev commented on code in PR #8179:
URL: https://github.com/apache/arrow-rs/pull/8179#discussion_r2292424051


##########
parquet-variant-compute/src/variant_get/mod.rs:
##########
@@ -377,53 +581,114 @@ mod test {
     ///   typed_value: Int32Array,
     /// }
     /// ```
-    fn shredded_int32_variant_array() -> ArrayRef {
-        // At the time of writing, the `VariantArrayBuilder` does not support 
shredding.
-        // so we must construct the array manually.  see 
https://github.com/apache/arrow-rs/issues/7895
-        let (metadata, string_value) = {
-            let mut builder = parquet_variant::VariantBuilder::new();
-            builder.append_value("n/a");
-            builder.finish()
+    macro_rules! numeric_partially_shredded_variant_array_fn {
+        ($func:ident, $array_type:ident, $primitive_type:ty) => {
+            fn $func() -> ArrayRef {
+                // At the time of writing, the `VariantArrayBuilder` does not 
support shredding.
+                // so we must construct the array manually.  see 
https://github.com/apache/arrow-rs/issues/7895
+                let (metadata, string_value) = {
+                    let mut builder = parquet_variant::VariantBuilder::new();
+                    builder.append_value("n/a");
+                    builder.finish()
+                };
+
+                let nulls = NullBuffer::from(vec![
+                    true,  // row 0 non null
+                    false, // row 1 is null
+                    true,  // row 2 non null
+                    true,  // row 3 non null
+                ]);
+
+                // metadata is the same for all rows
+                let metadata = 
BinaryViewArray::from_iter_values(std::iter::repeat_n(&metadata, 4));
+
+                // See 
https://docs.google.com/document/d/1pw0AWoMQY3SjD7R4LgbPvMjG_xSCtXp3rZHkVp9jpZ4/edit?disco=AAABml8WQrY
+                // about why row1 is an empty but non null, value.
+                let values = BinaryViewArray::from(vec![
+                    None,                // row 0 is shredded, so no value
+                    Some(b"" as &[u8]),  // row 1 is null, so empty value 
(why?)
+                    Some(&string_value), // copy the string value "N/A"
+                    None,                // row 3 is shredded, so no value
+                ]);
+
+                let typed_value = $array_type::from(vec![
+                    Some(<$primitive_type>::try_from(34u8).unwrap()), // row 0 
is shredded, so it has a value
+                    None,                                             // row 1 
is null, so no value
+                    None, // row 2 is a string, so no typed value

Review Comment:
   Not sure why `cargo fmt` aligns the comment like this.



##########
parquet-variant-compute/src/variant_get/output/variant.rs:
##########
@@ -18,11 +18,52 @@
 use crate::variant_get::output::OutputBuilder;
 use crate::{VariantArray, VariantArrayBuilder};
 use arrow::array::{Array, ArrayRef, AsArray, BinaryViewArray};
-use arrow::datatypes::Int32Type;
+use arrow::datatypes::{
+    Float16Type, Float32Type, Float64Type, Int16Type, Int32Type, Int64Type, 
Int8Type, UInt16Type,
+    UInt32Type, UInt64Type, UInt8Type,
+};
 use arrow_schema::{ArrowError, DataType};
 use parquet_variant::{Variant, VariantPath};
 use std::sync::Arc;
 
+macro_rules! cast_perfectly_shredded_primitive {
+    ($typed_value:expr, $variant_array:expr, $arrow_type:ty, $data_type:expr) 
=> {{
+        let mut array_builder = VariantArrayBuilder::new($variant_array.len());
+        let primitive_array = $typed_value.as_primitive::<$arrow_type>();
+        for i in 0..$variant_array.len() {
+            if primitive_array.is_null(i) {
+                array_builder.append_null();
+            } else {
+                let value = primitive_array.value(i);
+                array_builder.append_variant(Variant::from(value));
+            }
+        }

Review Comment:
   Yeah, re-using that macro after it merges would work. 



-- 
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