cashmand commented on code in PR #9663:
URL: https://github.com/apache/arrow-rs/pull/9663#discussion_r3087944473
##########
parquet-variant-compute/src/shred_variant.rs:
##########
@@ -321,12 +328,19 @@ impl<'a> VariantToShreddedArrayVariantRowBuilder<'a> {
// If the variant is not an array, typed_value must be null.
// If the variant is an array, value must be null.
match variant {
- Variant::List(list) => {
+ Variant::List(ref list) => {
self.nulls.append_non_null();
- self.value_builder.append_null();
- self.typed_value_builder
- .append_value(&Variant::List(list))?;
- Ok(true)
+
+ // With `safe` cast option set to false, appending list of
wrong size to
+ // `typed_value_builder` of type `FixedSizeList` will result
in an error. In such a
+ // case, the provided list should be appended to the
`value_builder.
Review Comment:
Hi, I worked on the shredding spec, and the intent of that line of the spec
was to apply to any array, not just one that perfectly matches the shredding
schema. For example, in a query with `try_cast(v as array<variant>)`, an engine
would be entitled to only fetch the `typed_value` column for parquet, and
produce null for all of the rows where `typed_value` is null. This would break
if `value` could contain arrays.
--
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]
