alamb commented on code in PR #21357:
URL: https://github.com/apache/datafusion/pull/21357#discussion_r3035547898


##########
datafusion/datasource-parquet/src/metadata.rs:
##########
@@ -146,32 +134,69 @@ impl<'a> DFParquetMetadata<'a> {
             return Ok(Arc::clone(cached_parquet.parquet_metadata()));
         }
 
-        let mut reader =
-            
ParquetMetaDataReader::new().with_prefetch_hint(*metadata_size_hint);
+        let file_size = self.object_meta.size;
+        let mut decoder = ParquetMetaDataPushDecoder::try_new(file_size)
+            .map_err(DataFusionError::from)?;

Review Comment:
   Shouldn't `?` do the error conversion directly ? I think this could just be 
   ```rust
   let mut decoder = ParquetMetaDataPushDecoder::try_new(file_size)?;
   ``` 
   (and also below)



##########
datafusion/datasource-parquet/src/metadata.rs:
##########
@@ -146,32 +134,69 @@ impl<'a> DFParquetMetadata<'a> {
             return Ok(Arc::clone(cached_parquet.parquet_metadata()));
         }
 
-        let mut reader =
-            
ParquetMetaDataReader::new().with_prefetch_hint(*metadata_size_hint);
+        let file_size = self.object_meta.size;
+        let mut decoder = ParquetMetaDataPushDecoder::try_new(file_size)
+            .map_err(DataFusionError::from)?;
 
         #[cfg(feature = "parquet_encryption")]
-        if let Some(decryption_properties) = decryption_properties {
-            reader = reader
-                
.with_decryption_properties(Some(Arc::clone(decryption_properties)));
+        if let Some(decryption_properties) = &self.decryption_properties {
+            decoder = decoder
+                
.with_file_decryption_properties(Some(Arc::clone(decryption_properties)));
         }
 
-        if cache_metadata && file_metadata_cache.is_some() {
+        if cache_metadata && self.file_metadata_cache.is_some() {
             // Need to retrieve the entire metadata for the caching to be 
effective.
-            reader = reader.with_page_index_policy(PageIndexPolicy::Optional);
+            decoder = 
decoder.with_page_index_policy(PageIndexPolicy::Optional);
+        } else {
+            decoder = decoder.with_page_index_policy(PageIndexPolicy::Skip);
         }
 
-        let metadata = Arc::new(
-            reader
-                .load_and_finish(fetch, object_meta.size)
+        // If we have a size hint, prefetch that many bytes from the end of 
the file

Review Comment:
   this is cool to do this prefetching explicitly 



-- 
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]

Reply via email to