Skip to content

[Variant] Minor: Add comments to tickets for follow on items #8092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 13, 2025
Merged

[Variant] Minor: Add comments to tickets for follow on items #8092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9aeb54b
[Variant] Minor: Add tickets for follow ons
alamb Aug 7, 2025
52c0f83
Merge branch 'main' into alamb/links_to_tickets
alamb Aug 13, 2025
3c20f4e
Update parquet-variant-compute/src/variant_get/output/variant.rs
alamb Aug 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion parquet-variant-compute/src/variant_array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ impl VariantArray {
#[derive(Debug)]
pub enum ShreddingState {
// TODO: add missing state where there is neither value nor typed_value
// https://github.com/apache/arrow-rs/issues/8088
// Missing { metadata: BinaryViewArray },
/// This variant has no typed_value field
Unshredded {
Expand Down Expand Up @@ -342,7 +343,7 @@ fn typed_value_to_variant(typed_value: &ArrayRef, index: usize) -> Variant<'_, '
_ => {
// We shouldn't panic in production code, but this is a
// placeholder until we implement more types
// TODO tickets: XXXX
// https://github.com/apache/arrow-rs/issues/8091
debug_assert!(
false,
"Unsupported typed_value type: {:?}",
Expand Down
6 changes: 3 additions & 3 deletions parquet-variant-compute/src/variant_get/output/primitive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ impl<'a, T: ArrowPrimitiveVariant> OutputBuilder for PrimitiveOutputBuilder<'a,

// if the typed value is null, decode the variant and extract the value
if typed_value.is_null(i) {
// todo follow path
// TODO follow path
// https://github.com/apache/arrow-rs/issues/8086
let variant = variant_array.value(i);
let Some(value) = T::from_variant(&variant) else {
if self.cast_options.safe {
Expand Down Expand Up @@ -137,6 +138,7 @@ impl<'a, T: ArrowPrimitiveVariant> OutputBuilder for PrimitiveOutputBuilder<'a,
Ok(typed_value.clone())
} else {
// TODO: try to cast the typed_value to the desired type?
// https://github.com/apache/arrow-rs/issues/8086
Err(ArrowError::NotYetImplemented(format!(
"variant_get fully_shredded as {:?} with typed_value={:?} is not implemented yet",
self.as_type.data_type(),
Expand All @@ -162,5 +164,3 @@ impl ArrowPrimitiveVariant for Int32Type {
variant.as_int32()
}
}

// todo for other primitive types
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drive by cleanup (to quote @houqp from a long time ago)

2 changes: 2 additions & 0 deletions parquet-variant-compute/src/variant_get/output/variant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ impl<'a> OutputBuilder for VariantOutputBuilder<'a> {
}
}
dt => {
// https://github.com/apache/arrow-rs/issues/8086
return Err(ArrowError::NotYetImplemented(format!(
"variant_get fully_shredded with typed_value={dt} is not implemented yet",
)));
Expand Down Expand Up @@ -104,6 +105,7 @@ impl<'a> OutputBuilder for VariantOutputBuilder<'a> {
}
}
dt => {
// https://github.com/apache/arrow-rs/issues/8087
return Err(ArrowError::NotYetImplemented(format!(
"variant_get fully_shredded with typed_value={dt} is not implemented yet",
)));
Expand Down
Loading