Skip to content

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

Open

Description

@PiotrSrebrny

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.

Activity

  1. nathanb9 commented on Apr 20, 2026

    @nathanb9
    Contributor

    @PiotrSrebrny Thanks for reporting. Just FYI looks like the sections below "To Reproduced" are in the code block not in MD format so its harder to read your ticket.

    Seems to be missing parser logic or a wrong assumption like you said because currently just doesn't emit the select from the window expressions into the result.

    I don't see why we can't just build the SELECT list from input columns + window expressions if nothing is projected above it which is flagged by already_projected().

  2. nathanb9 commented on Apr 20, 2026

    @nathanb9
    Contributor

    take

  3. PiotrSrebrny commented on Apr 21, 2026

    @PiotrSrebrny
    Author

    Thanks @nathanb9 for taking this. I fixed the formatting.

  4. nathanb9 commented on May 1, 2026

    @nathanb9
    Contributor

    @PiotrSrebrny If you can review PR that would be nice. Its right above ^

  5. PiotrSrebrny commented on May 11, 2026

    @PiotrSrebrny
    Author

    Hi @nathanb9 I am back from my vacations now, I will do the review. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions