raghav-reglobe commented on code in PR #10598:
URL: https://github.com/apache/arrow-rs/pull/10598#discussion_r3751040345
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -1152,17 +1152,13 @@ fn typed_value_to_variant(typed_value: &ArrayRef,
index: usize) -> Result<Varian
}
// todo other types here (note this is very similar to
cast_to_variant.rs)
// so it would be great to figure out how to share this code
- _ => {
- // We shouldn't panic in production code, but this is a
- // placeholder until we implement more types
- // https://github.com/apache/arrow-rs/issues/8091
- debug_assert!(
- false,
- "Unsupported typed_value type: {}",
- typed_value.data_type()
- );
- Ok(Variant::Null)
- }
+ //
+ // Returning Variant::Null here would silently misreport stored values
+ // as null in release builds
(https://github.com/apache/arrow-rs/issues/10597)
+ _ => Err(ArrowError::NotYetImplemented(format!(
Review Comment:
Yes — #10597 (linked in the description as "Closes #10597", and the new
comment in this arm points there too). The #8091 link is the old code's — it
goes away with the removed `debug_assert` block. #8091 was closed with the
placeholder still in place, which is why #10597 was filed to track the
remaining conversions (shredded objects/lists).
--
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]
