You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "adamreeve (via GitHub)" <gi...@apache.org> on 2025/05/07 04:20:30 UTC

[PR] Refactor Parquet DecryptionPropertiesBuilder [arrow-rs]

adamreeve opened a new pull request, #7477:
URL: https://github.com/apache/arrow-rs/pull/7477

   # Which issue does this PR close?
   
   Closes #7476
   
   # Rationale for this change
    
   Tidies up code to make it more maintainable and also makes invalid uses of the builder (ie. setting column keys directly when using a key retriever) cause a compilation error rather than a runtime error.
   
   https://github.com/apache/arrow-rs/pull/7286#discussion_r2010618162 suggested changing `DecryptionPropertiesBuilder` to be an enum, but that would require changing `DecryptionPropertiesBuilder::with_column_key` to return a `Result`, and I figured it would be better to keep the API mostly source compatible and make it a compilation error to call this when a key retriever is used rather than a runtime error.
   
   # What changes are included in this PR?
   
   Refactors how `FileDecryptionProperties` are built and adds a new `DecryptionPropertiesBuilderWithRetriever` type.
   
   # Are there any user-facing changes?
   
   Yes, this is a breaking change to the Parquet encryption API, but should be source compatible unless users were directly calling `DecryptionPropertiesBuilder::new_with_key_retriever` rather than `FileDecryptionProperties::with_key_retriever`. Parquet encryption is also a recently added experimental feature so I don't imagine this will affect many users.
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor Parquet DecryptionPropertiesBuilder [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7477:
URL: https://github.com/apache/arrow-rs/pull/7477#issuecomment-2860296122

   Looks to me like there is a conflict that needs to be resolved, but after that it should be good to go


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor Parquet DecryptionPropertiesBuilder [arrow-rs]

Posted by "adamreeve (via GitHub)" <gi...@apache.org>.
adamreeve commented on PR #7477:
URL: https://github.com/apache/arrow-rs/pull/7477#issuecomment-2860504756

   > Looks to me like there is a conflict that needs to be resolved, but after that it should be good to go
   
   This is resolved now, I had to account for the new `footer_signature_verification` option that was added.
   
   >  It would be nice to test this even with a mock key retriever.
   
   There are already some tests that cover this, eg: https://github.com/apache/arrow-rs/blob/fb72b8f7cb805f7abfaf325d19b3fb92aeb247ab/parquet/tests/encryption/encryption.rs#L252
   Given this PR is only a refactor I don't think there are any extra tests needed.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor Parquet DecryptionPropertiesBuilder [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7477:
URL: https://github.com/apache/arrow-rs/pull/7477#issuecomment-2862635594

   Thanks again @adamreeve 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor Parquet DecryptionPropertiesBuilder [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #7477:
URL: https://github.com/apache/arrow-rs/pull/7477


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org