mrutunjay-kinagi opened a new pull request, #58976:
URL: https://github.com/apache/spark/pull/58976
### What changes were proposed in this pull request?
`OptimizeWindowFunctions` rewrites `first(col)` over a suitable row frame
into
`nth_value(col, 1)`. The rule matched the aggregate as
`AggregateExpression(first: First, _, _, _, _)`, binding the fourth field,
`filter: Option[Expression]`, to a wildcard and then discarding it.
`NthValue` is a
window function and has nowhere to carry a `FILTER` clause, so the rewrite
dropped
the filter.
This PR binds that field and adds `filter.isEmpty` to the rule's guard, so a
filtered `first` is left alone and evaluated on the regular `First` path.
### Why are the changes needed?
`first_value(...) FILTER (WHERE ...)` over a window silently returns wrong
results.
The filter is ignored and the first unfiltered value of the frame is
returned, with
no error. Using the example from SPARK-59608:
```sql
SELECT id, grp, v, flag,
first_value(v) FILTER (WHERE flag) OVER (
PARTITION BY grp
ORDER BY id
ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
) AS first_flagged
FROM VALUES
(1, 1, 10, false),
(2, 1, 20, true),
(3, 1, 30, false),
(1, 2, 40, false),
(2, 2, 50, true)
AS t(id, grp, v, flag)
ORDER BY grp, id;
```
Before this change `first_flagged` is `10, 10, 10, 40, 50`. Excluding only
`OptimizeWindowFunctions` gives the correct `NULL, 20, 20, NULL, 50`, so the
results
depend on whether the optimizer rule fires.
The rule is enabled by default, the frames it matches are ordinary ones, and
the
query produces no error, so affected users get incorrect numbers with
nothing to
indicate a problem.
### Does this PR introduce _any_ user-facing change?
Yes. `first` / `first_value` with a `FILTER` clause over a `ROWS` frame
starting at
`UNBOUNDED PRECEDING` now honours the filter and returns the first value
among the
rows that satisfy it, or `NULL` when no row in the frame does. Previously
the filter
was ignored and the first value of the frame was returned regardless. Queries
without a `FILTER` clause are unaffected and still get the `nth_value`
rewrite.
This corrects results that were wrong, so no configuration is added to
restore the
old behaviour.
### How was this patch tested?
Two new tests:
- `OptimizeWindowFunctionsSuite`: a plan-level case asserting the rule
leaves the
window expression unchanged when the aggregate carries a filter, covering
both
frames the rule rewrites (`UNBOUNDED PRECEDING AND CURRENT ROW` and
`UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING`).
- `DataFrameWindowFunctionsSuite`: an end-to-end case running the query
above and
checking the returned values for both of those frames.
Existing `OptimizeWindowFunctionsSuite` cases cover the unfiltered rewrite
and the
frames the rule must not touch.
### Was this patch authored or co-authored using generative AI tooling?
Assisted by Claude Code (Opus 5), used to investigate the reported
behaviour, identify
the cause in `OptimizeWindowFunctions`, and write the tests.
--
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]
