apache / apache/datafusion

plan_to_sql drops window expressions for Window(Aggregate) plans without Projection

Open
#21,746 5 comments 1 reaction 1 assignee Claimed by @nathanb9 View on GitHub
bug
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Describe the bug

plan_to_sql drops window expressions when the logical plan has Window directly on top of Aggregate (without an intermediate Projection).

Given this logical plan shape:
```
WindowAggr: [row_number() ORDER BY [gas.time ASC NULLS FIRST] ... AS row_idx]
Aggregate: groupBy=[[gas.time]], aggr=[[avg(gas.value) AS avg_n]]
TableScan: gas
```
the unparser generates SQL that only contains the aggregate part (for example, avg_n and time) and omits row_number() entirely.

This produces semantically incorrect SQL for the plan output schema.

### To Reproduce

A direct reproducer is to build Window(Aggregate(TableScan)) manually:
```
#[test]
fn test_unparse_window_over_aggregate_without_projection() -> datafusion_common::Result<()> {
use arrow_schema::{DataType, Field, Schema};
use datafusion::prelude::{col, ExprFunctionExt};
use datafusion_expr::logical_plan::table_scan;
use datafusion_functions_aggregate::expr_fn::avg;
use datafusion_functions_window::expr_fn::row_number;
use datafusion_sql::unparser::Unparser;
let schema = Schema::new(vec![
Field::new("time", DataType::Int64, false),
Field::new("value", DataType::Float64, true),
]);
let window_expr = row_number()
.order_by(vec![col("time").sort(true, true)])
.build()?
.alias("row_idx");
let plan = table_scan(Some("gas"), &schema, None)?
.aggregate(
vec![col("time")],
vec![avg(col("value")).alias("avg_n")],
)?
.window(vec![window_expr])?
.build()?;
let sql = Unparser::default().plan_to_sql(&plan)?.to_string();
// This assertion currently fails: row_number() is dropped from SQL
assert!(
sql.to_lowercase().contains("row_number"),
"window expression was dropped during unparse: {sql}"
);
Ok(())
}
```
### Expected behavior

The generated SQL should preserve window semantics, e.g. either:

• include window expression directly in SELECT:
• ..., row_number() OVER (...) AS row_idx
• or preserve plan boundaries with a derived subquery where needed.

It should not silently omit window outputs from a Window node.

### Additional context

Root cause

In datafusion/sql/src/unparser/plan.rs, select_to_sql_recursively handles LogicalPlan::Window by delegating directly to its input with the comment that window nodes are handled with Projection.

That assumption breaks for plans where Window is not wrapped by Projection (common in manually-built or optimizer-rewritten plans). In that case:

• reconstruct_select_statement is never called (it only runs in the Projection arm),
• Window expressions are never added back to the SELECT list,
• unparse falls through to aggregate-only SELECT output.

So Window(Aggregate(...)) without projection loses window output during unparsing.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.