scovich commented on code in PR #9681:
URL: https://github.com/apache/arrow-rs/pull/9681#discussion_r3306930658
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -171,6 +187,16 @@ fn shredded_get_path(
}
ShreddedPathStep::Missing => {
let num_rows = input.len();
+ if
as_field.is_some_and(Field::has_valid_extension_type::<VariantType>) {
+ let all_nulls =
Some(arrow::buffer::NullBuffer::from(vec![false; num_rows]));
+ let arr = VariantArray::from_parts(
+ input.metadata_field().clone(),
Review Comment:
If it's all-NULL, why do we need to propagate the `metadata` column?
Couldn't it be all-NULL as well, because the physical metadata values don't
matter if the whole struct is masked by NULL?
##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -214,30 +240,32 @@ fn shredded_get_path(
//
// For shredded/partially-shredded targets (`typed_value` present),
recurse into each field
// separately to take advantage of deeper shredding in child fields.
- if let DataType::Struct(fields) = as_field.data_type() {
- if target.typed_value_field().is_none() {
- return shred_basic_variant(target, VariantPath::default(),
Some(as_field));
- }
-
- let children = fields
- .iter()
- .map(|field| {
- shredded_get_path(
- &target,
- &[VariantPathElement::from(field.name().as_str())],
- Some(field),
- cast_options,
- )
- })
- .collect::<Result<Vec<_>>>()?;
-
- let struct_nulls = target.nulls().cloned();
+ if !as_field.has_valid_extension_type::<VariantType>() {
+ if let DataType::Struct(fields) = as_field.data_type() {
+ if target.typed_value_field().is_none() {
+ return shred_basic_variant(target, VariantPath::default(),
Some(as_field));
+ }
- return Ok(Arc::new(StructArray::try_new(
- fields.clone(),
- children,
- struct_nulls,
- )?));
+ let children = fields
+ .iter()
+ .map(|field| {
+ shredded_get_path(
+ &target,
+ &[VariantPathElement::from(field.name().as_str())],
+ Some(field),
+ cast_options,
+ )
+ })
+ .collect::<Result<Vec<_>>>()?;
+
+ let struct_nulls = target.nulls().cloned();
+
+ return Ok(Arc::new(StructArray::try_new(
+ fields.clone(),
+ children,
+ struct_nulls,
Review Comment:
Why not just
```suggestion
return Ok(Arc::new(StructArray::try_new(
fields.clone(),
children,
target.nulls().cloned(),
```
--
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]
