Describe the bug
Aggregate::try_new_with_schema marks every grouping expression nullable whenever a grouping set is present (datafusion/expr/src/logical_plan/plan.rs:4102-4113), so an aggregate's logical schema says nullable where the physical schema says not-null.
Measured on 3266eaa91 over t(c utf8 NOT NULL, a i64 NOT NULL), where ? marks a nullable field:
SELECT c, a, count(*) FROM t GROUP BY GROUPING SETS ((c,a),(c))
logical : c?, a?, count(*)
physical: c, a?, count(*)
SELECT c, a, count(*) FROM t GROUP BY GROUPING SETS ((c,a))
logical : c?, a?, count(*)
physical: c, a, count(*)
c belongs to every set of the first query, and the second query has a single set, so no row is padded with a null in either column.
The physical side computes this per expression in PhysicalGroupBy::group_fields (datafusion/physical-plan/src/aggregates/mod.rs:576-590) as group_expr_nullable || expr.nullable(input_schema)? — set-absence on one side, the expression's own nullability on the other. That is the rule it has used since #12256, which changed physical-plan and left the logical side as it was.
Expected behavior
A grouping expression is nullable in the logical schema when the input makes it nullable, or when some grouping set leaves it out.
Before writing that, I would like to know what you expect it to move. DataFrame::schema() narrows for these queries, so optimizer rules keyed on nullability, EXPLAIN snapshots and sqllogictest results can shift with it, and crates outside the repo may read the wider schema today. If it is a change you want, I am glad to open the PR.
Additional context
Found while comparing relation-level schema derivation across Substrait implementations: for a Substrait plan with grouping sets DataFusion answers with the logical schema, so a consumer that compares schemas sees the wider one while execution produces the narrower.
Describe the bug
Aggregate::try_new_with_schemamarks every grouping expression nullable whenever a grouping set is present (datafusion/expr/src/logical_plan/plan.rs:4102-4113), so an aggregate's logical schema says nullable where the physical schema says not-null.Measured on
3266eaa91overt(c utf8 NOT NULL, a i64 NOT NULL), where?marks a nullable field:cbelongs to every set of the first query, and the second query has a single set, so no row is padded with a null in either column.The physical side computes this per expression in
PhysicalGroupBy::group_fields(datafusion/physical-plan/src/aggregates/mod.rs:576-590) asgroup_expr_nullable || expr.nullable(input_schema)?— set-absence on one side, the expression's own nullability on the other. That is the rule it has used since #12256, which changedphysical-planand left the logical side as it was.Expected behavior
A grouping expression is nullable in the logical schema when the input makes it nullable, or when some grouping set leaves it out.
Before writing that, I would like to know what you expect it to move.
DataFrame::schema()narrows for these queries, so optimizer rules keyed on nullability, EXPLAIN snapshots and sqllogictest results can shift with it, and crates outside the repo may read the wider schema today. If it is a change you want, I am glad to open the PR.Additional context
Found while comparing relation-level schema derivation across Substrait implementations: for a Substrait plan with grouping sets DataFusion answers with the logical schema, so a consumer that compares schemas sees the wider one while execution produces the narrower.