UBarney commented on code in PR #15954:
URL: https://github.com/apache/datafusion/pull/15954#discussion_r2105771450
##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -733,13 +733,33 @@ impl AggregateExec {
&self.input_order_mode
}
- fn statistics_inner(&self) -> Result<Statistics> {
+ fn statistics_inner(&self, child_statistics: Statistics) ->
Result<Statistics> {
// TODO stats: group expressions:
// - once expressions will be able to compute their own stats, use it
here
// - case where we group by on a column for which with have the
`distinct` stat
// TODO stats: aggr expression:
// - aggregations sometimes also preserve invariants such as min,
max...
- let column_statistics = Statistics::unknown_column(&self.schema());
+
+ let column_statistics = {
+ // self.schema: [<group by exprs>, <aggregate exprs>]
+ let mut column_statistics =
Statistics::unknown_column(&self.schema());
+
+ for (idx, (expr, _)) in self.group_by.expr.iter().enumerate() {
Review Comment:
> If we adjust the order of group by exprs in the test, does it still match?
Yes.
<details>
#[tokio::test]
async fn test_agg_order() -> Result<()> {
let scan = create_scan_exec_with_statistics(None, Some(2)).await;
let scan_schema = scan.schema();
let group_by = PhysicalGroupBy::new_single(vec![
(
binary(
lit(1),
Operator::Plus,
col("id", &scan_schema)?,
&scan_schema,
)?,
"expr".to_string(),
),
(col("id", &scan_schema)?, "id".to_string()),
]);
let aggr_expr = vec![AggregateExprBuilder::new(count_udaf(),
vec![lit(1)])
.schema(Arc::clone(&scan_schema))
.alias(String::from("COUNT(c)"))
.build()
.map(Arc::new)?];
let aggregate_exec_partial = Arc::new(AggregateExec::try_new(
AggregateMode::Partial,
group_by.clone(),
aggr_expr.clone(),
vec![None],
Arc::clone(&scan),
scan_schema.clone(),
)?);
let p0_statistics =
aggregate_exec_partial.partition_statistics(Some(0))?;
let expected_p0_statistics = Statistics {
num_rows: Precision::Inexact(2),
total_byte_size: Precision::Absent,
column_statistics: vec![
ColumnStatistics::new_unknown(),
ColumnStatistics {
null_count: Precision::Absent,
max_value: Precision::Exact(ScalarValue::Int32(Some(4))),
min_value: Precision::Exact(ScalarValue::Int32(Some(3))),
sum_value: Precision::Absent,
distinct_count: Precision::Absent,
},
ColumnStatistics::new_unknown(),
],
};
assert_eq!(&p0_statistics, &expected_p0_statistics);
let expected_p1_statistics = Statistics {
num_rows: Precision::Inexact(2),
total_byte_size: Precision::Absent,
column_statistics: vec![
ColumnStatistics::new_unknown(),
ColumnStatistics {
null_count: Precision::Absent,
max_value: Precision::Exact(ScalarValue::Int32(Some(2))),
min_value: Precision::Exact(ScalarValue::Int32(Some(1))),
sum_value: Precision::Absent,
distinct_count: Precision::Absent,
},
ColumnStatistics::new_unknown(),
],
};
let p1_statistics =
aggregate_exec_partial.partition_statistics(Some(1))?;
assert_eq!(&p1_statistics, &expected_p1_statistics);
validate_statistics_with_data(
aggregate_exec_partial.clone(),
vec![
ExpectedStatistics::NonEmpty(3, 4, 2),
ExpectedStatistics::NonEmpty(1, 2, 2),
],
1,
)
.await?;
let agg_final = Arc::new(AggregateExec::try_new(
AggregateMode::FinalPartitioned,
group_by.as_final(),
aggr_expr.clone(),
vec![None],
aggregate_exec_partial.clone(),
aggregate_exec_partial.schema(),
)?);
let p0_statistics = agg_final.partition_statistics(Some(0))?;
assert_eq!(&p0_statistics, &expected_p0_statistics);
let p1_statistics = agg_final.partition_statistics(Some(1))?;
assert_eq!(&p1_statistics, &expected_p1_statistics);
validate_statistics_with_data(
agg_final.clone(),
vec![
ExpectedStatistics::NonEmpty(3, 4, 2),
ExpectedStatistics::NonEmpty(1, 2, 2),
],
1,
)
.await?;
Ok(())
}
</details>
> The idx will be 1
`col.index()` will stay same (`0`)
https://github.com/apache/datafusion/blob/2d801940c3cb0cec3209aa890688590ded791865/datafusion/physical-expr/src/expressions/column.rs#L167-L169
--
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
