Skip to content

Conversation

@klion26
Copy link
Member

@klion26 klion26 commented Jul 6, 2025

Which issue does this PR close?

Add tests for large variant list.

Rationale for this change

Add tests for larget vairant lists

What changes are included in this PR?

This PR adds three tests for large variant lists.

  • one for total child size between 2^8 and 2^16
  • one for total child size between 2^16 and 2^24
  • one for total child size between 2^24 and 2^32

all the tests will verify the is_large, offset_size and the content of the list.

Are these changes tested?

Yes

Are there any user-facing changes?

Non

@github-actions github-actions bot added the parquet Changes to the parquet crate label Jul 6, 2025
@klion26
Copy link
Member Author

klion26 commented Jul 6, 2025

@alamb Please help review this when you're free, thanks.

@klion26 klion26 force-pushed the 7820-test-for-large-variant-list branch from e35d448 to c65058c Compare July 7, 2025 05:56
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @klion26 -- this looks great

I left a comment about checking the list offsets. Let us know what you think

cc @friendlymatthew and @scovich

@klion26
Copy link
Member Author

klion26 commented Jul 8, 2025

@alamb, thanks for the review. I've addressed the comment. Please take another look when you're free. Thank you.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @klion26 -- I think this looks great ❤️

A very nice contribution

@alamb alamb merged commit d7dae2c into apache:main Jul 10, 2025
12 checks passed
@klion26 klion26 deleted the 7820-test-for-large-variant-list branch July 11, 2025 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Variant] Tests for creating "large" VariantLists

2 participants