Skip to content

[Variant] Fix the variant shred logic#10157

Open
klion26 wants to merge 1 commit into
apache:mainfrom
klion26:fix_variant_shred
Open

[Variant] Fix the variant shred logic#10157
klion26 wants to merge 1 commit into
apache:mainfrom
klion26:fix_variant_shred

Conversation

@klion26

@klion26 klion26 commented Jun 18, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

What changes are included in this PR?

  • Seperate the logic for variant_shred and variant_get by introduce shred flag for row builder
  • Move some logic in Variant::as_xxx to type_conversion, and these moved code will be used when variant_get, variant_shred will use Variant::as_xxx
  • After the change, when shredding a variant, int8 can be treated as int8/int16/int32/int64, but bool/.../float can't be treated as int*, float can be shredded into float/double, but int can't be shredded to float/double, decimal4 can be shredded into decimal32/64/128(_, _), but int/float can't be shredded into decimal
  • Add a test to cover that shred can/can't be shredded to some datatype in test_variant_type_shredded_correctly

Are these changes tested?

Yes, added some tests to cover the logic

Are there any user-facing changes?

Yes, some Variant::as_xx logic have been changed.

@github-actions github-actions Bot added the parquet-variant parquet-variant* crates label Jun 18, 2026
@klion26 klion26 force-pushed the fix_variant_shred branch from 32a3f09 to aca457f Compare June 18, 2026 06:39
Previously, variant_shred and variant_get reuse the same code path,
causing type mismatches (e.g., bool was incorrectly shredded as int).
This commit separates the shred path and ensures each type is converted correctly.
@klion26 klion26 force-pushed the fix_variant_shred branch from aca457f to 003214c Compare June 18, 2026 06:55

@klion26 klion26 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@scovich @sdf-jkl Please help review this when you're free, thanks.

impl_primitive_from_variant!(datatypes::Float32Type, as_f32);
impl_primitive_from_variant!(datatypes::Float64Type, as_f64);
impl_primitive_from_variant!(datatypes::Date32Type, as_naive_date, |v| {
enum NumericKind {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These code are mainly moved from Variant.rs, the change here was adding the parameter shred for from_variant and variant_to_unscaled_decimal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parquet-variant parquet-variant* crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix the shred logic in parquet-variant

1 participant