Skip to content

Conversation

scovich
Copy link
Contributor

@scovich scovich commented Sep 16, 2025

Which issue does this PR close?

We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax.

  • Closes #NNN.

Rationale for this change

Variant metadata only "matters" for variant values that contain objects. Especially in unit tests, it is common for a given variant value to have an empty variant metadata -- often one created separately and replicated across many rows.

What changes are included in this PR?

Define new constants, EMPTY_VARIANT_METADATA_BYTES and EMPTY_VARIANT_METADATA, which are exactly what they sound like.

Are these changes tested?

New doc tests, and several unit tests were updated to use it as well.

Are there any user-facing changes?

New constants

@github-actions github-actions bot added the parquet-variant parquet-variant* crates label Sep 16, 2025
@scovich
Copy link
Contributor Author

scovich commented Sep 16, 2025

CC @alamb @sdf-jkl

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.

Thanks @scovich -- makes sense to me

cc @codephage2020 and @klion26 and @mbrobbel

/// let metadata_bytes = metadata_builder.into_inner();
/// assert_eq!(&metadata_bytes, EMPTY_VARIANT_METADATA_BYTES);
/// ```
pub const EMPTY_VARIANT_METADATA_BYTES: &[u8] = &[1, 0, 0];
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@alamb alamb merged commit df8b38e into apache:main Sep 16, 2025
12 checks passed
@klion26
Copy link
Member

klion26 commented Sep 17, 2025

This makes it more readable; thank you for the improvement.

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.

3 participants