apache / apache/datafusion

Logical schema of an aggregate over GROUPING SETS is more nullable than the physical schema

Open
#24,968 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### 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.

Contributor guide

Open the contributing guide

Research direction

Read Aggregate::try_new_with_schema in datafusion/expr/src/logical_plan/plan.rs:4102-4113 and compare its grouping-set nullability with PhysicalGroupBy::group_fields in datafusion/physical-plan/src/aggregates/mod.rs:576-590. Trace the affected schema and EXPLAIN or sqllogictest coverage, then verify that logical nullability matches input nullability plus grouping-set absence without unintended snapshot changes.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, sql
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.