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


##########
datafusion/core/src/datasource/physical_plan/parquet/row_group_filter.rs:
##########
@@ -356,20 +356,24 @@ impl<'a> RowGroupPruningStatistics<'a> {
         &'a self,
         column: &'b Column,
     ) -> Result<StatisticsConverter<'a>> {
-        StatisticsConverter::try_new(&column.name, self.arrow_schema, 
self.parquet_schema)
+        Ok(StatisticsConverter::try_new(
+            &column.name,
+            self.arrow_schema,
+            self.parquet_schema,
+        )?)
     }
 }
 
 impl<'a> PruningStatistics for RowGroupPruningStatistics<'a> {
     fn min_values(&self, column: &Column) -> Option<ArrayRef> {
         self.statistics_converter(column)
-            .and_then(|c| c.row_group_mins(self.metadata_iter()))
+            .and_then(|c| Ok(c.row_group_mins(self.metadata_iter())?))

Review Comment:
   Thank you for the suggestion.
   
   I played around with a few alternatives and I concluded they were not easier 
to understand, so I plan to leave it as is. If you feel strongly I will make a 
follow on PR to change.
   
   I couldn't figure out a way to use `into()`
   
   ## Option 1:  using `map_err` 
   
   ```rust
       fn min_values(&self, column: &Column) -> Option<ArrayRef> {
           self.statistics_converter(column)
               .and_then(|c| {
                   c.row_group_mins(self.metadata_iter())
                       .map_err(DataFusionError::from)
               })
               .ok()
       }
   ```
   
   ## Option 2: discard error earlier with `ok()`
   
   Now there are two nested `ok()`s
   
   ```rust
       fn min_values(&self, column: &Column) -> Option<ArrayRef> {
           self.statistics_converter(column)
               .ok()
               .and_then(|c| c.row_group_mins(self.metadata_iter()).ok())
       }
   ```



-- 
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-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to