Skip to content

fix(variant): reject FixedSizeList shredding - #10639

Open
cakeni wants to merge 1 commit into
apache:mainfrom
cakeni:fix/variant-fixed-size-list
Open

fix(variant): reject FixedSizeList shredding#10639
cakeni wants to merge 1 commit into
apache:mainfrom
cakeni:fix/variant-fixed-size-list

Conversation

@cakeni

@cakeni cakeni commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

The Variant shredding spec does not define FixedSizeList as a valid shredded typed_value. Rather than extending support beyond the spec, reject it consistently at the shred_variant boundary.

What changes are included in this PR?

  • Reject DataType::FixedSizeList when selecting a shredding type.
  • Keep the existing VariantArray::try_new rejection unchanged.
  • Cover FixedSizeList in the invalid shredding type test set.

Are these changes tested?

  • cargo test -p parquet-variant-compute --lib (362 passed)
  • cargo fmt --all -- --check
  • git diff --check

Are there any user-facing changes?

shred_variant now returns an error for FixedSizeList instead of producing a shredded representation that other Variant APIs reject.

@github-actions github-actions Bot added the parquet-variant parquet-variant* crates label Aug 11, 2026
@Jefffrey Jefffrey added the enhancement Any new improvement worthy of a entry in the changelog label Aug 27, 2026
@Jefffrey Jefffrey changed the title fix(variant): support FixedSizeList across shredded APIs feat(variant): support FixedSizeList across shredded APIs Aug 27, 2026

@Jefffrey Jefffrey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the docs dont mention support for fixed size list: https://arrow.apache.org/docs/format/CanonicalExtensions.html#parquet-variant

should we be supporting this?

@cakeni
cakeni force-pushed the fix/variant-fixed-size-list branch from f2da410 to 1801209 Compare August 28, 2026 07:11
@cakeni cakeni changed the title feat(variant): support FixedSizeList across shredded APIs fix(variant): reject FixedSizeList shredding Aug 28, 2026
@cakeni

cakeni commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

You're right. FixedSizeList isn't part of the Variant shredding spec. I changed the PR to reject it in shred_variant instead of extending the reader APIs, and added it to the invalid shredding type coverage.

@Jefffrey Jefffrey added bug and removed enhancement Any new improvement worthy of a entry in the changelog labels Aug 28, 2026
@Jefffrey

Copy link
Copy Markdown
Contributor

cc @klion26 @sdf-jkl

@sdf-jkl

sdf-jkl commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

This is a more difficult one because it is still an open question and has a long history. My head is spinning after thinking about it all day.

The Arrow canonical extension documentation does not list FixedSizeList as a supported typed_value type. However, I could not find FixedSizeList being considered or mentioned during the discussion in apache/arrow#47456, so I am hesitant to treat its omission as a deliberate decision to reject it.

At the Parquet level, an Arrow FixedSizeList is still encoded as an ordinary three-level LIST. The fixed length exists only in the Arrow schema metadata, so accepting it is not obviously a question of Parquet Variant specification compliance.

The relevant arrow-rs history is:

Using ListLikeArray for cast_to_variant makes sense: Variant value encoding has only ordinary arrays, so an Arrow FixedSizeList becomes a Variant array and its fixed length is discarded.

I think reusing the same abstraction to determine which types unshred_variant accepts was a mistake. ListLikeArray only says that an Arrow array can expose child values and an element range; it does not say that every implementing type is a valid shredded typed_value.

If we decide that FixedSizeList is not a supported Variant typed_value, then both shred_variant and unshred_variant should reject it. Keeping the FSL unshredding branch does not currently provide backwards-compatible reading because VariantArray::try_new already rejects an FSL typed_value before unshred_variant can be called.

Alternatively, if we want a lenient-reader policy for data previously produced by arrow-rs, we should explicitly accept or normalize FSL on read and add a persisted read/unshred test. Leaving only the existing internal unshredding branch does not accomplish that.

DuckDB provides the clearest comparison. Its fixed-length list equivalent, ARRAY, is explicitly rejected as a shredding target. Its Variant reader has no FSL-specific unshredding path: the physical Parquet LIST is inferred as a variable-length DuckDB LIST, and Arrow-specific fixed-size metadata is not restored. It therefore unshreds the value as an ordinary Variant array without preserving or validating a fixed length.

@cakeni

cakeni commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

I’d keep this strict for now: reject FixedSizeList in shred_variant and remove the unreachable FSL branch in unshred_variant, since VariantArray::try_new already rejects it. If we want to support older data written by arrow-rs, we can handle that separately by normalizing FSL to a regular list when reading it.

@sdf-jkl

sdf-jkl commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

we can handle that separately by normalizing FSL to a regular list when reading it.

💯

)?;
VariantToShreddedVariantRowBuilder::Array(typed_value_builder)
}
DataType::FixedSizeList(..) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the separation of this branch and underscore our expected behavior? Seems the error is the same.

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

Labels

bug parquet-variant parquet-variant* crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Variant] Reconcile FixedSizeList support across shredded Variant APIs

4 participants