laskoviymishka commented on code in PR #3144:
URL: https://github.com/apache/iceberg-rust/pull/3144#discussion_r4389614389
##########
crates/iceberg/src/expr/visitors/row_group_metrics_evaluator.rs:
##########
@@ -497,32 +497,17 @@ impl BoundPredicateVisitor for
RowGroupMetricsEvaluator<'_> {
return ROW_GROUP_MIGHT_MATCH;
}
- let Some((stats, primitive_type)) =
self.stats_and_type_for_field_id(field_id)? else {
- return ROW_GROUP_MIGHT_MATCH;
- };
-
- if let Some(lower_bound) =
get_parquet_stat_min_as_datum(&primitive_type, stats)? {
- if lower_bound.is_nan() {
- // NaN indicates unreliable bounds. See the
InclusiveMetricsEvaluator docs for more.
- return ROW_GROUP_MIGHT_MATCH;
- }
-
- if !literals.iter().any(|datum| datum.ge(&lower_bound)) {
- // if all values are less than lower bound, rows cannot match.
- return ROW_GROUP_CANT_MATCH;
- }
- }
-
- if let Some(upper_bound) =
get_parquet_stat_max_as_datum(&primitive_type, stats)? {
- if upper_bound.is_nan() {
- // NaN indicates unreliable bounds. See the
InclusiveMetricsEvaluator docs for more.
- return ROW_GROUP_MIGHT_MATCH;
- }
-
- if !literals.iter().any(|datum| datum.le(&upper_bound)) {
- // if all values are greater than upper bound, rows cannot
match.
- return ROW_GROUP_CANT_MATCH;
- }
+ let lower_bound = self.min_value(field_id)?;
+ let upper_bound = self.max_value(field_id)?;
Review Comment:
Optional: could this keep the single `stats_and_type_for_field_id` lookup
from #3254? `min_value` and `max_value` each resolve the stats again, so `in`
now does it twice while `eq` does it once. Results don't change. The suggestion
compiles and passes the `expr::visitors` tests.
```suggestion
let Some((stats, primitive_type)) =
self.stats_and_type_for_field_id(field_id)? else {
return ROW_GROUP_MIGHT_MATCH;
};
let lower_bound = get_parquet_stat_min_as_datum(&primitive_type,
stats)?;
let upper_bound = get_parquet_stat_max_as_datum(&primitive_type,
stats)?;
```
--
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]
