Repository navigation
Conversation
manuzhang
marked this pull request as ready for review
November 7, 2026 16:44
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Relates to apache/datafusion-comet#6246.
Rationale for this change
Applications that adapt cached Parquet metadata need to replace file-level key/value entries, for example an Arrow schema hint, before constructing a reader. The existing public constructors require rebuilding
FileMetaDataandParquetMetaDatato make this change. That loses private encryption metadata and the file decryptor, so this approach cannot safely handle encrypted files.A setter on the existing metadata builder lets callers update those entries while preserving the state required to read the file. A downstream use is Comet's per-file schema adaptation, which currently has to exclude encrypted files from its metadata rewrite.
What changes are included in this PR?
Add
ParquetMetaDataBuilder::set_key_value_metadata(Option<Vec<KeyValue>>). It replaces the complete list, withNoneremoving the entries.Arc::make_mutgives shared file metadata copy-on-write behavior and preserves the other file metadata, row groups, page indexes and decryptor. The update affects only in-memory metadata.Are these changes tested?
A unit test covers adding, replacing and removing entries, including an explicitly empty list. It checks that cached metadata remains unchanged, the other file metadata (including encryption fields when enabled) is preserved, and row groups and page indexes remain shared.
An encrypted-read regression covers encrypted and plaintext footers. It updates already-loaded metadata, verifies its page indexes, and reads the encrypted data without supplying decryption keys again. This checks that the cached decryptor survives the update.
Are there any significant user-facing changes?
This is an additive public API. Existing method signatures and behavior are unchanged.
AI assistance: Codex drafted the implementation and regression tests and ran local validation.