Skip to content

Commit

Permalink
Physical-plan counterpart for DF apache#11256.
Browse files Browse the repository at this point in the history
In apache#11256 it started inferring, for logical plans only,
that count() aggregation does not return nulls.
However, it did not introduce the counterpart inference for physical plans.
Somehow, this discrepancy did not get detected by DF tests, but we run into it with, e.g.
`sdf run -q "SELECT count(*) FROM (VALUES (3, 'c')) as T(c, x) GROUP BY c "`

The fix here is ad hoc.
We should keep an eye on apache#11274 where the goal seems to make UDAFs to communicate nullability of their result.  If done right, that should have affect both logical and physical plans.
  • Loading branch information
vgapeyev authored and findepi committed Oct 7, 2024
1 parent 85dbc97 commit d595069
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion datafusion/physical-expr-common/src/aggregate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,8 @@ impl AggregateExpr for AggregateFunctionExpr {
}

fn field(&self) -> Result<Field> {
Ok(Field::new(&self.name, self.data_type.clone(), true))
let nullable = self.fun.name() != "count";
Ok(Field::new(&self.name, self.data_type.clone(), nullable))
}

fn create_accumulator(&self) -> Result<Box<dyn Accumulator>> {
Expand Down

0 comments on commit d595069

Please sign in to comment.