timsaucer opened a new issue, #24885:
URL: https://github.com/apache/datafusion/issues/24885
### 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`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
