jayzhan211 commented on code in PR #12296:
URL: https://github.com/apache/datafusion/pull/12296#discussion_r1759627130
##########
datafusion/physical-optimizer/src/aggregate_statistics.rs:
##########
@@ -140,155 +138,24 @@ fn take_optimizable_column_and_table_count(
agg_expr: &AggregateFunctionExpr,
stats: &Statistics,
) -> Option<(ScalarValue, String)> {
- let col_stats = &stats.column_statistics;
- if is_non_distinct_count(agg_expr) {
- if let Precision::Exact(num_rows) = stats.num_rows {
- let exprs = agg_expr.expressions();
- if exprs.len() == 1 {
- // TODO optimize with exprs other than Column
- if let Some(col_expr) =
- exprs[0].as_any().downcast_ref::<expressions::Column>()
- {
- let current_val = &col_stats[col_expr.index()].null_count;
- if let &Precision::Exact(val) = current_val {
- return Some((
- ScalarValue::Int64(Some((num_rows - val) as i64)),
- agg_expr.name().to_string(),
- ));
- }
- } else if let Some(lit_expr) =
- exprs[0].as_any().downcast_ref::<expressions::Literal>()
- {
- if lit_expr.value() == &COUNT_STAR_EXPANSION {
- return Some((
- ScalarValue::Int64(Some(num_rows as i64)),
- agg_expr.name().to_string(),
- ));
- }
- }
- }
- }
- }
- None
-}
-
-/// If this agg_expr is a min that is exactly defined in the statistics,
return it.
-fn take_optimizable_min(
- agg_expr: &AggregateFunctionExpr,
- stats: &Statistics,
-) -> Option<(ScalarValue, String)> {
- if let Precision::Exact(num_rows) = &stats.num_rows {
- match *num_rows {
- 0 => {
- // MIN/MAX with 0 rows is always null
- if is_min(agg_expr) {
- if let Ok(min_data_type) =
- ScalarValue::try_from(agg_expr.field().data_type())
- {
- return Some((min_data_type,
agg_expr.name().to_string()));
- }
- }
- }
- value if value > 0 => {
- let col_stats = &stats.column_statistics;
- if is_min(agg_expr) {
- let exprs = agg_expr.expressions();
- if exprs.len() == 1 {
- // TODO optimize with exprs other than Column
- if let Some(col_expr) =
-
exprs[0].as_any().downcast_ref::<expressions::Column>()
- {
- if let Precision::Exact(val) =
- &col_stats[col_expr.index()].min_value
- {
- if !val.is_null() {
- return Some((
- val.clone(),
- agg_expr.name().to_string(),
- ));
- }
- }
- }
- }
- }
- }
- _ => {}
+ if !agg_expr.is_distinct() {
+ if let Some((val, name)) =
take_optimizable_value_from_statistics(agg_expr, stats)
+ {
+ return Some((val, name));
}
}
None
}
/// If this agg_expr is a max that is exactly defined in the statistics,
return it.
-fn take_optimizable_max(
+fn take_optimizable_value_from_statistics(
agg_expr: &AggregateFunctionExpr,
stats: &Statistics,
) -> Option<(ScalarValue, String)> {
- if let Precision::Exact(num_rows) = &stats.num_rows {
- match *num_rows {
- 0 => {
- // MIN/MAX with 0 rows is always null
- if is_max(agg_expr) {
- if let Ok(max_data_type) =
- ScalarValue::try_from(agg_expr.field().data_type())
- {
- return Some((max_data_type,
agg_expr.name().to_string()));
- }
- }
- }
- value if value > 0 => {
- let col_stats = &stats.column_statistics;
- if is_max(agg_expr) {
- let exprs = agg_expr.expressions();
- if exprs.len() == 1 {
- // TODO optimize with exprs other than Column
- if let Some(col_expr) =
-
exprs[0].as_any().downcast_ref::<expressions::Column>()
- {
- if let Precision::Exact(val) =
- &col_stats[col_expr.index()].max_value
- {
- if !val.is_null() {
- return Some((
- val.clone(),
- agg_expr.name().to_string(),
- ));
- }
- }
- }
- }
- }
- }
- _ => {}
- }
- }
- None
+ let value = agg_expr.fun().value_from_stats(
Review Comment:
We have `agg_expr.is_distinct()` here, so you can differentiate distinct
count and non-distinct case.
Maybe we can have `StatisticsArgs` similar to `AccumulatorArgs`.
```rust
pub struct StatisticsArgs<'a> {
statistics: &'a Statistics,
return_type: &'a DataType,
/// Whether the aggregate function is distinct.
///
/// ```sql
/// SELECT COUNT(DISTINCT column1) FROM t;
/// ```
pub is_distinct: bool,
/// The physical expression of arguments the aggregate function takes.
pub exprs: &'a [Arc<dyn PhysicalExpr>],
}
```
--
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]
