Repository navigation
feat(encryption): [11/N] Read encrypted Parquet data files - #4309
Conversation
| from pyarrow.parquet import encryption | ||
|
|
||
| MIN_PYARROW_VERSION_SUPPORTING_PARQUET_DECRYPTION = "25.0.0" | ||
| if version.parse(pyarrow.__version__) < version.parse(MIN_PYARROW_VERSION_SUPPORTING_PARQUET_DECRYPTION): |
There was a problem hiding this comment.
Not 100% sure what the blessed way to do this is...
There was a problem hiding this comment.
yea i think we do this pattern in a few places already.
its not pretty but its what we got 🤷
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
I pushed up a small fix in 1ae3dd3
- Combined
_get_file_formatand_get_data_file_formatinto a single_create_arrow_read_format(file_format, key_metadata, **kwargs), withkey_metadatarequired so callers can't leave it out - Removed
lru_cacheso per-file keys aren't kept in memory (building a format costs ~1µs) - Detect PyArrow decryption support by trying the import instead of comparing version numbers (no
packagingimport, and dev builds work) - Reject encrypted non-Parquet files, including Puffin deletion vectors, with the file path in the error
- Updated tests to match
LMK if you agree, and feel free to make changes
There was a problem hiding this comment.
thanks for keeping this small 😄
as i mentioned in the other pr, would be good to add a script to be able to regenerate these test files
There was a problem hiding this comment.
Let me open an issue so I don’t forget
|
I’m happy with the changes, thanks @kevinjqliu |
|
Will rebase on main |
A data file's key_metadata holds its Parquet encryption key and AAD prefix, so scans decrypt with pyarrow's direct key API when it is set, as iceberg-rust does. That API needs pyarrow 25.
Merge format selection and Parquet decryption into _create_arrow_read_format, dropping the lru_cache so per-file keys are not retained. Detect PyArrow decryption support by import instead of parsing the version. Reject encrypted non-Parquet files, including deletion vectors, with the file path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| def _create_arrow_read_format(file_format: FileFormat, key_metadata: bytes | None, **kwargs: Any) -> ds.FileFormat: | ||
| if file_format == FileFormat.PARQUET: | ||
| if key_metadata is not None: | ||
| kwargs["decryption_properties"] = _parquet_decryption_properties(key_metadata) |
There was a problem hiding this comment.
decryption_properties documentation
https://arrow.apache.org/docs/python/generated/pyarrow.parquet.ParquetFile.html
|
|
||
| @lru_cache | ||
| def _get_file_format(file_format: FileFormat, **kwargs: dict[str, Any]) -> ds.FileFormat: | ||
| def _create_arrow_read_format(file_format: FileFormat, key_metadata: bytes | None, **kwargs: Any) -> ds.FileFormat: |
There was a problem hiding this comment.
we could consider making key_metadata required so future callers wont forget to include it
There was a problem hiding this comment.
This is sorted now right?
1ae3dd3 to
0b2f6e5
Compare
|
|
||
| @lru_cache | ||
| def _get_file_format(file_format: FileFormat, **kwargs: dict[str, Any]) -> ds.FileFormat: | ||
| def _create_arrow_read_format(file_format: FileFormat, key_metadata: bytes | None, **kwargs: Any) -> ds.FileFormat: |
|
thanks again @xanderbailey 🚀 |
Working towards #3222
Rationale for this change
We need to be able to read data files encrypted with Parquet Modular Encryption.
key_metadatain its manifest entry holds the plaintext key and AAD prefix for that file. The manifest holding it is itself encrypted, so this doesn't need theEncryptionManageror the KMS.key_metadatais set, scans build the Parquet file format with decryption properties from PyArrow's direct key API,pyarrow.parquet.encryption.create_decryption_properties(GH-47435: [Python][Parquet] Add direct key encryption/decryption API arrow#49667). This matches iceberg-rust, which builds arrow-rsFileDecryptionPropertiesfrom the key and AAD prefix ([9/N] feat(encryption) [9/N] Read encrypted parquet data-files iceberg-rust#2584).The direct key API was added in PyArrow 25, so reading an encrypted file on an older PyArrow raises an
ImportErrorasking for 25.0.0 or later. The minimum PyArrow version for everything else is unchanged.Are these changes tested?
Yes. A Parquet data file encrypted by iceberg-rust is checked in under
tests/table/encryption, and reading it with its key metadata returns its rows. There's also a round trip of an encrypted data file with an encrypted position delete file. Reads fail with a wrong key, a wrong AAD prefix, missing key metadata, or key metadata on a plaintext file.Are there any user-facing changes?
No. Nothing changes for unencrypted tables.