zhuqi-lucas opened a new pull request, #25865:
URL: https://github.com/apache/datafusion/pull/25865
## Which issue does this PR close?
- Closes #25864.
## Rationale for this change
A predicate that depends only on a window's `PARTITION BY` keys is constant
within every partition, so applying it below the window drops whole partitions
and leaves every surviving row's window value unchanged. `push_down_filter`
already does this, but only for plain column keys.
The key set is built by mapping each partition key through
`qualified_name()` into a `Column`, so `PARTITION BY a + b` becomes a column
literally named `"a + b"`. A predicate on `a + b` reads the real columns `a`
and `b`, which never match that synthesised name, so it stays above the window.
The optimization simply does not exist for expression keys such as `a + b`,
`NULLIF(c, '')` or `COALESCE(x, y)`.
It also affects *mixed* predicates. With `PARTITION BY year, num * num`:
```sql
SELECT * FROM (
SELECT year, num, SUM(num) OVER (PARTITION BY year, num * num) AS s FROM
t
)
WHERE year = '2021' OR num * num > 4
```
the predicate is constant within every partition and safe to push, but its
column refs are `{year, num}` against a key set of `{year, "num * num"}`, so
`num` is not found and the whole conjunct is kept.
## What changes are included in this PR?
**Keep a volatile predicate above the window** (first commit, independent of
the rest)
A volatile predicate that reads no columns, such as `random() < 0.5`,
satisfies the subset test vacuously and was pushed below the window, where it
changes which rows the window function sees. The aggregate arm already drops
volatile group expressions before the equivalent check; this does the same
here. Split out as its own commit so it can be taken separately.
**Match against the key expressions** (second commit)
The key set holds the partition key `Expr`s rather than names synthesised
from them, and each conjunct is walked: a subtree exactly equal to one of the
keys counts as read in full, and the predicate is rejected only on reaching a
`Column` that no key covered.
This is a strict generalization. For a plain column key, the reference to
that column is itself a subtree equal to the key, so every predicate pushed
today is still pushed; expression keys and mixed predicates are added on top.
Matching is structural, so it stays conservative: a predicate written `b + a`
does not match a key written `a + b` and is left in place.
No predicate rewriting is involved. The existing comment in that arm notes
that a window partition expression, unlike an aggregate group expression, is
not exposed as a standalone column, so there is nothing to rewrite `a + b`
into. The predicate is pushed unchanged.
## Are these changes tested?
Yes, in `push_down_filter`'s unit tests:
- `filter_expression_move_window`: a predicate on `PARTITION BY a + b` is
now pushed. This replaces `filter_expression_keep_window`, which pinned the
previous behavior.
- `filter_move_window_mixed_column_and_expression_keys`: `PARTITION BY a, a
+ b` with `a > 1 OR a + b > 10` is pushed, the case neither the old matching
nor an expression-key-only rule could see.
- `filter_keep_window_column_underlying_expression_key`: an expression key
does not make the columns it reads pushable on their own, so `a > 10` under
`PARTITION BY a + b` stays above.
- `filter_volatile_keep_window`: the volatility case above. Verified it
fails without the guard.
The existing window tests (`filter_move_window`,
`filter_move_partial_window`, `filter_order_keep_window`,
`filter_multiple_windows_common_partitions`,
`filter_multiple_windows_disjoint_partitions`, and the rest) are unchanged and
pass, which is the evidence for the "strict generalization" claim.
Still to do before this leaves draft: confirm the `sqllogictest` plan churn.
More pushdown means some window plans change, and those are expected updates
rather than regressions, but I want to post the actual diff here rather than
assert it.
## Are there any user-facing changes?
Better plans for window queries filtered on expression partition keys, and a
correctness fix for volatile predicates over windows. No API changes.
--
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]
