apache / apache/datafusion

Support first_value/last_value aggregates as sliding window aggregates (blocks FILTER + windowed first/last value)

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

Description

### Is your feature request related to a problem or challenge?

`FILTER (WHERE …)` is only accepted on *aggregate* window functions:

```
Error during planning: FILTER clause can only be used with aggregate window functions.
Found in 'last_value(d.v) FILTER (WHERE d.keep) ORDER BY [d.t ASC NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW'
```

In SQL, `last_value(v) OVER (…)` always resolves to the `last_value` **window UDF** (`datafusion/functions-window/src/nth_value.rs`), so filtered first/last-value over a window is unreachable from SQL entirely. (PostgreSQL rejects `FILTER` on non-aggregate window functions too, so that part is not a deviation.)

DataFusion does have a working alternative: the `last_value` **aggregate UDAF** can be used as a window function through the DataFrame API, and it honors `FILTER` with correct semantics:

```rust
Expr::from(WindowFunction::new(last_value_udaf(), vec![col("v")]))
.order_by(vec![col("t").sort(true, false)])
.filter(col("keep"))
.build()?
```

```
+---+----+-------+----+
| t | v | keep | w |
+---+----+-------+----+
| 1 | 10 | true | 10 |
| 2 | 20 | false | 10 |
| 3 | | true | | <- kept by filter, value is NULL -> result NULL (correct)
| 4 | 40 | false | |
+---+----+-------+----+
```

But that path is second-class today, and breaks as soon as the frame is anything but a simple ever-expanding one:

```
This feature is not implemented: Aggregate can not be used as a sliding accumulator because
`retract_batch` is not implemented: last_value(?table?.v) ORDER BY [?table?.t ASC NULLS LAST]
ROWS BETWEEN 1 PRECEDING AND CURRENT ROW
```

The same error is reachable indirectly with a plain `UNBOUNDED PRECEDING .. CURRENT ROW` frame: when the physical optimizer reverses the window to avoid a sort, the reversed frame becomes `ROWS CURRENT ROW .. UNBOUNDED FOLLOWING`, which is not `is_ever_expanding()`, so `get_reverse_expr` builds a `SlidingAggregateWindowExpr` that then demands `retract_batch`. (The reversal path currently fails earlier with a separate renaming bug, #24884; with that fixed locally, the query proceeds to this error.)

The usual `CASE` rewrite does **not** reproduce `FILTER` semantics when filtered-in values can be NULL:

```sql
-- RESPECT NULLS: excluded rows become NULL and are then taken as the "last value"
last_value(CASE WHEN keep THEN v END) OVER (ORDER BY t)
-- t=2 (keep=false) -> NULL, expected 10

-- IGNORE NULLS: conflates "excluded by filter" with "included but NULL"
last_value(CASE WHEN keep THEN v END) IGNORE NULLS OVER (ORDER BY t)
-- t=3 (keep=true, v IS NULL) -> 10, expected NULL
```

So for filtered first/last-value over windows there is currently no correct formulation that also works with sliding frames.

### Describe the solution you'd like

Implement `retract_batch` for the `first_value` / `last_value` aggregate accumulators (`datafusion/functions-aggregate/src/first_last.rs`) so they can be used as sliding window aggregates, the way `array_agg` was handled in #21957.

Where a true retract is not possible in general, the alternative is to make the reversal/frame classification avoid `SlidingAggregateWindowExpr` when the reversed frame is plain in the reverse direction, so the common `UNBOUNDED PRECEDING .. CURRENT ROW` case does not silently degrade into a sliding accumulator.

### Describe alternatives you've considered

- `CASE WHEN` rewrite with `IGNORE NULLS` / `RESPECT NULLS` — shown above, does not match `FILTER` semantics when filtered-in values may be NULL.
- Allowing `FILTER` on the `first_value` / `last_value` window UDFs — would diverge from the SQL standard and PostgreSQL, and would duplicate filtering logic already present in the aggregate path.
- Routing `last_value(...) FILTER (...) OVER (...)` in the SQL planner to the aggregate UDAF instead of the window UDF — plausible follow-up, but only useful once the aggregate path handles sliding/reversed frames, which is what this issue asks for.

### Additional context

Related: #24884 (window reversal renames the output field for aggregate-UDAF window functions; blocks reaching this error via the reversal path).

Reproduced on `main` (`d7b8e4fc1`).

Contributor guide

Open the contributing guide

Research direction

Start in datafusion/functions-aggregate/src/first_last.rs and inspect the first_value and last_value aggregate accumulators, then reproduce the sliding-frame error described in the issue. Check the related window reversal path in #24884. Done means filtered first/last-value aggregates work with sliding and reversed frames, including cases where filtered-in values are NULL.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, sql
Domain
data-engineering, databases
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.