Self-CR things

Signed-off-by: Adam Gutglick <adam@spiraldb.com>

Author: AdamGS

encodings/parquet-variant/src/array.rs

@@ -29,6 +29,7 @@ use vortex_array::builtins::ArrayBuiltins;
 use vortex_array::dtype::DType;
 use vortex_array::dtype::FieldName;
 use vortex_array::dtype::FieldNames;
+use vortex_array::dtype::Nullability;
 use vortex_array::smallvec::smallvec;
 use vortex_array::validity::Validity;
 use vortex_array::vtable::child_to_validity;
@@ -161,12 +162,13 @@ pub(crate) fn core_storage_without_typed_value(
 pub(crate) fn logical_shredded_from_parquet_typed_value(
     metadata: &ArrayRef,
     typed_value: ArrayRef,
+    ctx: &mut ExecutionCtx,
 ) -> VortexResult<ArrayRef> {
     if let Some(list_array) = typed_value.as_opt::<List>() {
         // Lists keep their original offsets and validity; only the physical element
         // representation may need wrapper removal.
         let elements =
-            logical_shredded_from_parquet_field(metadata, list_array.elements().clone())?
+            logical_shredded_from_parquet_field(metadata, list_array.elements().clone(), ctx)?
                 .unwrap_or_else(|| list_array.elements().clone());
         return Ok(ListArray::try_new(
             elements,
@@ -189,7 +191,9 @@ pub(crate) fn logical_shredded_from_parquet_typed_value(
         .iter()
         .zip(struct_array.iter_unmasked_fields())
     {
-        if let Some(logical_field) = logical_shredded_from_parquet_field(metadata, field.clone())? {
+        if let Some(logical_field) =
+            logical_shredded_from_parquet_field(metadata, field.clone(), ctx)?
+        {
             names.push(FieldName::from(name.as_ref()));
             fields.push(logical_field);
         }
@@ -211,6 +215,7 @@ pub(crate) fn logical_shredded_from_parquet_typed_value(
 fn logical_shredded_from_parquet_field(
     metadata: &ArrayRef,
     field: ArrayRef,
+    ctx: &mut ExecutionCtx,
 ) -> VortexResult<Option<ArrayRef>> {
     let Some(field_struct) = field.as_opt::<Struct>() else {
         return Ok(Some(field));
@@ -248,48 +253,49 @@ fn logical_shredded_from_parquet_field(
         let Some(value) = value else {
             // Fully shredded field: recurse through the typed subtree and expose its
             // logical shape directly.
-            return logical_shredded_from_parquet_typed_value(metadata, typed_value).map(Some);
+            return logical_shredded_from_parquet_typed_value(metadata, typed_value, ctx).map(Some);
         };
 
         // Partially shredded terminal object: keep raw `value` available as the nested
         // Variant core storage while exposing any typed children as nested `shredded`.
-        let validity = inferred_shredded_field_validity(Some(&value), Some(&typed_value))?;
+        let validity = inferred_shredded_field_validity(Some(&value), Some(&typed_value), ctx)?;
         let parquet_field =
             ParquetVariant::try_new(validity, metadata.clone(), Some(value), Some(typed_value))?;
         let shredded = parquet_field
             .typed_value_array()
             .cloned()
-            .map(|typed_value| logical_shredded_from_parquet_typed_value(metadata, typed_value))
+            .map(|typed_value| {
+                logical_shredded_from_parquet_typed_value(metadata, typed_value, ctx)
+            })
             .transpose()?;
         return VariantArray::try_new(core_storage_without_typed_value(&parquet_field)?, shredded)
             .map(|array| Some(array.into_array()));
     }
 
-    logical_shredded_from_parquet_typed_value(metadata, field).map(Some)
+    logical_shredded_from_parquet_typed_value(metadata, field, ctx).map(Some)
 }
 
 fn inferred_shredded_field_validity(
     value: Option<&ArrayRef>,
     typed_value: Option<&ArrayRef>,
+    ctx: &mut ExecutionCtx,
 ) -> VortexResult<Validity> {
     let len = value
         .or(typed_value)
         .map(ArrayRef::len)
         .vortex_expect("at least one shredded field child");
-    let validity = (0..len)
-        .map(|idx| {
-            let value_valid = value
-                .map(|value| value.validity()?.is_valid(idx))
-                .transpose()?
-                .unwrap_or(false);
-            let typed_valid = typed_value
-                .map(|typed_value| typed_value.validity()?.is_valid(idx))
-                .transpose()?
-                .unwrap_or(false);
-            Ok(value_valid || typed_valid)
-        })
-        .collect::<VortexResult<Vec<_>>>()?;
-    Ok(Validity::from_iter(validity))
+    let value_mask = value
+        .map(|value| value.validity()?.execute_mask(len, ctx))
+        .transpose()?;
+    let typed_mask = typed_value
+        .map(|typed_value| typed_value.validity()?.execute_mask(len, ctx))
+        .transpose()?;
+    let validity = match (value_mask, typed_mask) {
+        (Some(value_mask), Some(typed_mask)) => &value_mask | &typed_mask,
+        (Some(mask), None) | (None, Some(mask)) => mask,
+        (None, None) => unreachable!("at least one shredded field child"),
+    };
+    Ok(Validity::from_mask(validity, Nullability::Nullable))
 }
 
 /// Accessors and Arrow conversion for Parquet Variant storage arrays.

encodings/parquet-variant/src/vtable.rs

@@ -253,12 +253,12 @@ impl VTable for ParquetVariant {
         Ok(ArrayParts::new(self.clone(), dtype.clone(), len, EmptyArrayData).with_slots(slots))
     }
 
-    fn execute(array: Array<Self>, _ctx: &mut ExecutionCtx) -> VortexResult<ExecutionResult> {
+    fn execute(array: Array<Self>, ctx: &mut ExecutionCtx) -> VortexResult<ExecutionResult> {
         let shredded = array
             .typed_value_array()
             .cloned()
             .map(|typed_value| {
-                logical_shredded_from_parquet_typed_value(array.metadata_array(), typed_value)
+                logical_shredded_from_parquet_typed_value(array.metadata_array(), typed_value, ctx)
             })
             .transpose()?;
         let core_storage = core_storage_without_typed_value(&array)?;
@@ -350,16 +350,6 @@ mod tests {
             .unwrap()
     }
 
-    fn file_session() -> VortexSession {
-        let session = VortexSession::empty()
-            .with::<ArraySession>()
-            .with::<LayoutSession>()
-            .with::<RuntimeSession>();
-        vortex_file::register_default_encodings(&session);
-        session.arrays().register(ParquetVariant);
-        session
-    }
-
     #[test]
     fn test_execute_exposes_typed_value_as_canonical_shredded() -> VortexResult<()> {
         let metadata =
@@ -420,7 +410,13 @@ mod tests {
         let expected =
             ParquetVariant::from_arrow_variant(&ArrowVariantArray::try_new(&arrow_storage)?)?;
 
-        let session = file_session();
+        let session = VortexSession::empty()
+            .with::<ArraySession>()
+            .with::<LayoutSession>()
+            .with::<RuntimeSession>();
+        vortex_file::register_default_encodings(&session);
+        session.arrays().register(ParquetVariant);
+
         let mut bytes = ByteBufferMut::empty();
         session
             .write_options()

vortex-array/src/expr/exprs.rs

@@ -120,8 +120,8 @@ pub fn get_item(field: impl Into<FieldName>, child: Expression) -> Expression {
 
 /// Creates an expression that extracts a path from a Variant expression.
 ///
-/// When `dtype` is `None`, the expression returns nullable Variant values. When `dtype` is
-/// provided, the expression returns that dtype with nullable nullability.
+/// Missing paths, traversal mismatches, and failed casts return null. When `dtype` is `None`,
+/// results are nullable Variant values; otherwise results are nullable values of `dtype`.
 pub fn variant_get(
     child: Expression,
     path: impl Into<VariantPath>,

vortex-array/src/scalar_fn/fns/variant_get/mod.rs

@@ -31,7 +31,12 @@ use crate::scalar_fn::ExecutionArgs;
 use crate::scalar_fn::ScalarFnId;
 use crate::scalar_fn::ScalarFnVTable;
 
-/// Extracts a path from a Variant expression, optionally as a typed result.
+/// Extracts a field/index path from Variant values.
+///
+/// Missing paths, type mismatches while traversing, and failed casts produce nulls. Without a
+/// requested dtype, results are returned as nullable Variant values; with one, results are cast to
+/// that dtype with nullable nullability. Encodings may serve perfectly shredded paths directly,
+/// but must fall back to the core Variant value for paths not represented by shredded storage.
 #[derive(Clone)]
 pub struct VariantGet;