blackmwk commented on code in PR #2584:
URL: https://github.com/apache/iceberg-rust/pull/2584#discussion_r3576732658
##########
crates/iceberg/src/arrow/mod.rs:
##########
@@ -44,3 +44,38 @@ pub use partition_value_calculator::*;
/// Record batch partition splitter for partitioned tables
pub mod record_batch_partition_splitter;
pub use record_batch_partition_splitter::*;
+
+#[cfg(test)]
+pub(crate) mod test_utils {
Review Comment:
Please move this to a standalone file mode `test_utils`
##########
crates/iceberg/src/encryption/key_metadata.rs:
##########
@@ -97,7 +97,17 @@ impl StandardKeyMetadata {
/// Decodes from Java-compatible format.
pub fn decode(bytes: &[u8]) -> Result<Self> {
- _serde::StandardKeyMetadataV1::decode(bytes).map(Self::from)
+ let metadata =
_serde::StandardKeyMetadataV1::decode(bytes).map(Self::from)?;
+ // Validate the DEK is a usable AES key (16/24/32 bytes) up front, so a
+ // malformed key surfaces a clear error.
+ SecureKey::new(metadata.encryption_key.as_bytes()).map_err(|e| {
Review Comment:
If this is required, should we check in all constructor? Like `new` and
`From` implementation.
##########
crates/iceberg/src/arrow/reader/pipeline.rs:
##########
@@ -431,14 +437,44 @@ impl ArrowReader {
)
.with_parquet_read_options(parquet_read_options);
- let arrow_metadata = ArrowReaderMetadata::load_async(&mut reader,
Default::default())
+ let arrow_reader_options =
Self::build_arrow_reader_options(key_metadata)?;
+
+ let arrow_metadata = ArrowReaderMetadata::load_async(&mut reader,
arrow_reader_options)
.await
.map_err(|e| {
Error::new(ErrorKind::Unexpected, "Failed to load Parquet
metadata").with_source(e)
})?;
Ok((reader, arrow_metadata))
}
+
+ /// Builds `ArrowReaderOptions`, adding `FileDecryptionProperties` when
+ /// key metadata is present for Parquet Modular Encryption.
+ fn build_arrow_reader_options(key_metadata: Option<&[u8]>) ->
Result<ArrowReaderOptions> {
Review Comment:
I agree with @xanderbailey we could postpone the dynamic resolution approach
when necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
