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