Skip to content

Support for Arc<str> in ParquetRecordWriter derive macro#8973

Merged
alamb merged 12 commits intoapache:mainfrom
WalletConnect:master
Dec 11, 2025
Merged

Support for Arc<str> in ParquetRecordWriter derive macro#8973
alamb merged 12 commits intoapache:mainfrom
WalletConnect:master

Conversation

@heilhead
Copy link
Copy Markdown
Contributor

@heilhead heilhead commented Dec 9, 2025

Which issue does this PR close?

Rationale for this change

See #8972 for more info.

What changes are included in this PR?

Updates the derive macro implementation to include string serialization from Arc<str>.

Are these changes tested?

Yes. I've updated parquet_derive_test to include writing Arc<str>. And we've been using our fork with these changes in production for ~3 years.

Are there any user-facing changes?

Not sure, probably not?

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Dec 10, 2025

Thanks @heilhead -- this looks good to me

Copy link
Copy Markdown
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 @heilhead

@heilhead
Copy link
Copy Markdown
Contributor Author

@alamb Sorry for the ping, but can you please review again or merge it yourself? I've updated our branch with the upstream changes and now it requires another approval.

@heilhead
Copy link
Copy Markdown
Contributor Author

@alamb Sorry for pinging again, but can you please merge the PR? By the time I get your approval it's behind main, and after updating from upstream, the PR requires another approval. Not sure what to do.

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Dec 11, 2025

@alamb Sorry for pinging again, but can you please merge the PR? By the time I get your approval it's behind main, and after updating from upstream, the PR requires another approval. Not sure what to do.

I will handle merging this PR once CI passes -- there is nothing else you need to do.

Thank you for the contribution

@alamb alamb merged commit 6ff8cc4 into apache:main Dec 11, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for Arc<str> in ParquetRecordWriter derive macro

3 participants