etseidl commented on code in PR #6637: URL: https://github.com/apache/arrow-rs/pull/6637#discussion_r1869985184
########## parquet/src/file/footer.rs: ########## @@ -52,13 +53,16 @@ pub fn parse_metadata<R: ChunkReader>(chunk_reader: &R) -> Result<ParquetMetaDat /// Decodes [`ParquetMetaData`] from the provided bytes. /// /// Typically this is used to decode the metadata from the end of a parquet -/// file. The format of `buf` is the Thift compact binary protocol, as specified +/// file. The format of `buf` is the Thrift compact binary protocol, as specified /// by the [Parquet Spec]. /// /// [Parquet Spec]: https://github.com/apache/parquet-format#metadata #[deprecated(since = "53.1.0", note = "Use ParquetMetaDataReader::decode_metadata")] -pub fn decode_metadata(buf: &[u8]) -> Result<ParquetMetaData> { - ParquetMetaDataReader::decode_metadata(buf) +pub fn decode_metadata( Review Comment: I'm not sure we should be updating a deprecated function. If encryption is desired I'd say force use of the new API so we don't have to maintain this one. Just pass `None` to `ParquetMetaDataReader::decode_metadata`. ########## parquet/src/file/footer.rs: ########## @@ -52,13 +53,16 @@ pub fn parse_metadata<R: ChunkReader>(chunk_reader: &R) -> Result<ParquetMetaDat /// Decodes [`ParquetMetaData`] from the provided bytes. /// /// Typically this is used to decode the metadata from the end of a parquet -/// file. The format of `buf` is the Thift compact binary protocol, as specified +/// file. The format of `buf` is the Thrift compact binary protocol, as specified Review Comment: ❤️ ########## parquet/src/file/metadata/reader.rs: ########## @@ -342,8 +360,13 @@ impl ParquetMetaDataReader { mut fetch: F, file_size: usize, ) -> Result<()> { - let (metadata, remainder) = - Self::load_metadata(&mut fetch, file_size, self.get_prefetch_size()).await?; + let (metadata, remainder) = Self::load_metadata( + &mut fetch, + file_size, + self.get_prefetch_size(), + self.file_decryption_properties.clone(), Review Comment: Very minor nit: I understand that `file_decryption_properties` needs to be cloned eventually...just wondering if we could pass references down into `decode_metadata` and do the clone there where it's more obviously 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
