This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24866-b12c822eebfef3e7a21d30a2da648cfcef11f5c3 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit dd2b23e723b6d638764f2b5ccd7ac57ca0d62293 Author: Adam Reeve <[email protected]> AuthorDate: Wed Sep 9 08:16:59 2026 +0000 fix: check for key retriever in Parquet decryption properties conversion (#24866) ## Which issue does this PR close? - Closes #21634. ## Rationale for this change Previously an error when retrieving the footer key was used as an indication that a key retriever was used, but this isn't reliable. A key retriever implementation might choose to return a footer key even if no key metadata is provided. This could mean that a footer key is set in the converted decryption properties but column keys are not correctly converted. arrow-rs didn't previously expose whether a key retriever is used, but this was added in 59.0.0. ## What changes are included in this PR? Use the new `FileDecryptionProperties::uses_key_retriever` to determine whether `FileDecryptionProperties` uses a key retriever, rather than rely on getting an error when getting the footer key. ## What is the testing strategy for this PR? Updated existing unit test. ## Are there any user-facing changes? No, this changes an error message but should otherwise not affect users except if they had a key retriever implementation that would return footer keys without any metadata being provided. --- datafusion/common/src/config.rs | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/datafusion/common/src/config.rs b/datafusion/common/src/config.rs index f770cebf88..b48ee2248a 100644 --- a/datafusion/common/src/config.rs +++ b/datafusion/common/src/config.rs @@ -3695,11 +3695,20 @@ impl TryFrom<&Arc<FileDecryptionProperties>> for ConfigFileDecryptionProperties type Error = DataFusionError; fn try_from(f: &Arc<FileDecryptionProperties>) -> Result<Self> { + if f.uses_key_retriever() { + // Getting the keys is not possible without the key metadata from + // a Parquet file if a key retriever is used. + return Err( + DataFusionError::Configuration( + "Cannot convert FileDecryptionProperties that use a key retriever to ConfigFileDecryptionProperties".into() + ) + ); + } + let footer_key = f.footer_key(None).map_err(|e| { + // This shouldn't happen for FileDecryptionProperties that don't use a key retriever. DataFusionError::Configuration(format!( - "Could not retrieve footer key from FileDecryptionProperties. \ - Note that conversion to ConfigFileDecryptionProperties is not supported \ - when using a key retriever: {e}" + "Could not retrieve footer key from FileDecryptionProperties: {e}" )) })?; @@ -4470,14 +4479,10 @@ mod tests { #[cfg(feature = "parquet_encryption")] impl parquet::encryption::decrypt::KeyRetriever for ParquetEncryptionKeyRetriever { - fn retrieve_key(&self, key_metadata: &[u8]) -> parquet::errors::Result<Vec<u8>> { - if !key_metadata.is_empty() { - Ok(b"1234567890123450".to_vec()) - } else { - Err(parquet::errors::ParquetError::General( - "Key metadata not provided".to_string(), - )) - } + fn retrieve_key(&self, _key_metadata: &[u8]) -> parquet::errors::Result<Vec<u8>> { + // Ignore key metadata so we can verify that the key retriever isn't used + // even if it can provide a key without using metadata. + Ok(b"1234567890123450".to_vec()) } } @@ -4497,8 +4502,10 @@ mod tests { (&decryption_properties).try_into(); assert!(config_file_decryption_properties.is_err()); let err = config_file_decryption_properties.unwrap_err().to_string(); - assert!(err.contains("key retriever")); - assert!(err.contains("Key metadata not provided")); + assert_contains!( + err, + "Cannot convert FileDecryptionProperties that use a key retriever to ConfigFileDecryptionProperties" + ); } #[cfg(feature = "parquet")] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
