Skip to content

Arrow-avro Writer Dense Union support #8550

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

Jump to bottom
Merged
mbrobbel merged 17 commits into from
Oct 8, 2025
Merged

Arrow-avro Writer Dense Union support #8550

mbrobbel merged 17 commits into from
Oct 8, 2025

Conversation

@nathaniel-d-ef
Copy link
Contributor

@nathaniel-d-ef nathaniel-d-ef commented Oct 3, 2025

Which issue does this PR close?

Relates to:
#8348
#4886

Rationale for this change

This PR completes the efforts of @jecsand838, adding dense union support to the encoder side of the crate, along with four other minor extensions of existing time-related encoding.

Note: currently this PR is stacked behind #8546. Once that's merged this will be updated and will not include those changes.

What changes are included in this PR?

  • Dense union support for the writer
  • Tests

Are these changes tested?

  • A full round-trip test, reading in an existing union avro file and asserting that the output matches expectations
  • Unit tests covering new encoders.

Are there any user-facing changes?

Crate not yet public

@github-actions github-actions bot added arrow Changes to the arrow crate arrow-avro arrow-avro crate labels Oct 3, 2025
@jecsand838 jecsand838 mentioned this pull request Oct 6, 2025
Merged
@mbrobbel mbrobbel added this to the 57.0.0 milestone Oct 6, 2025
@mbrobbel mbrobbel mentioned this pull request Oct 7, 2025
Merged
mbrobbel
mbrobbel approved these changes Oct 7, 2025
View reviewed changes
Copy link
Member

@mbrobbel mbrobbel left a comment

Choose a reason for hiding this comment

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

Thanks @nathaniel-d-ef and @jecsand838

@jecsand838
Copy link
Contributor

jecsand838 commented Oct 7, 2025

@mbrobbel Just a heads up that @nathaniel-d-ef is out of office until early next week.

@alamb
Copy link
Contributor

alamb commented Oct 7, 2025 •
edited
Loading

@mbrobbel Just a heads up that @nathaniel-d-ef is out of office until early next week.

Thanks @jecsand838 -- to keep things moving do you think you would be able to implement the PR suggestions? I would normally do it myself, but I don't have the permissions to push to the elastiflow fork: https://github.com/elastiflow/arrow-rs

@nathaniel-d-ef
Copy link
Contributor Author

nathaniel-d-ef commented Oct 7, 2025 via email

@jecsand838
Copy link
Contributor

jecsand838 commented Oct 7, 2025

@mbrobbel Just a heads up that @nathaniel-d-ef is out of office until early next week.

Thanks @jecsand838 -- to keep things moving do you think you would be able to implement the PR suggestions? I would normally do it myself, but I don't have the permissions to push to the elastiflow fork: https://github.com/elastiflow/arrow-rs

Absolutely! Just pushed up those PR suggestions.

mbrobbel
mbrobbel approved these changes Oct 8, 2025
View reviewed changes
Copy link
Member

@mbrobbel mbrobbel left a comment

Choose a reason for hiding this comment

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

Thanks @jecsand838!

@mbrobbel mbrobbel merged commit f19bda3 into apache:main Oct 8, 2025
23 checks passed
@alamb
Copy link
Contributor

alamb commented Oct 8, 2025

woohoo!

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

Reviewers

@mbrobbel mbrobbel mbrobbel approved these changes

Assignees

No one assigned

Labels

arrow Changes to the arrow crate arrow-avro arrow-avro crate

Projects

None yet

Milestone

57.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@nathaniel-d-ef @jecsand838 @alamb @mbrobbel