viirya opened a new pull request, #59848:
URL: https://github.com/apache/spark/pull/59848
### What changes were proposed in this pull request?
`PushPredicateThroughNonJoin` reorders the conjuncts of a `Filter` over a
`Project` that has expensive (UDF, expensive regex) aliases. It does so in two
places, and neither checks `Expression.throwable`:
1. The projection split added by SPARK-55014
(`spark.sql.optimizer.splitProjectionForExpensiveFilters`). It orders condition
groups by how many not-yet-computed aliases they need, so a `raise_error` /
`assert_true` condition can be split below the conditions written before it.
2. The cheap/expensive partition added by SPARK-47672
(`spark.sql.optimizer.avoidDoubleFilterEval`). It pushes a cheap conjunct below
the projection, ahead of an expensive conjunct written before it.
This PR:
- keeps throwable conditions out of the split, using `deterministic &&
!throwable`, the same check `PushPredicateThroughJoin` uses;
- pushes a cheap throwable condition below the projection only when no
expensive condition comes before it;
- keeps the conditions left in the top filter in the order they were written.
Moving a non-throwable condition below a throwable one is still allowed,
since it can only reduce the rows the throwable condition sees.
`CombineFilters` and the join pushdown rules already behave this way.
### Why are the changes needed?
A throwable condition can be evaluated on rows that the `AND` short-circuit
would have kept from it, so a query that should succeed fails. With `f` and `g`
expensive UDFs:
```scala
spark.range(0, 10)
.select($"id", f($"id") as "f", g($"id") as "g")
.where($"f" + $"g" > 10 && assert_true($"g" > 5).isNull)
```
The `assert_true` group needs fewer aliases, so the split evaluates it
first, on every row, and the query fails with `USER_RAISED_EXCEPTION`. It
succeeds with `splitProjectionForExpensiveFilters=false`.
```scala
spark.range(0, 10)
.select($"id", f($"id") as "f")
.where($"f" > 5 && assert_true($"id" > 5).isNull)
```
Here the cheap `assert_true` condition is pushed below the projection, ahead
of `f > 5`. This fails whenever `avoidDoubleFilterEval` is true, whatever the
split config, and it is already in 4.2.0.
### Does this PR introduce _any_ user-facing change?
Yes. Queries like the ones above failed with `USER_RAISED_EXCEPTION` in
4.2.0 and later (the second shape), and on unreleased master / branch-4.x (the
first shape). They now succeed. Filters without throwable conditions are
optimized as before.
### How was this patch tested?
New tests:
- three tests in `FilterPushdownSuite` for the split, for a throwable
condition not blocking later conditions, and for the cheap pushdown path; all
three fail without this change;
- an end-to-end test in `DataFrameSuite` that runs both queries above over
every combination of whole-stage codegen, `splitProjectionForExpensiveFilters`
and `avoidDoubleFilterEval`.
All tests in `org.apache.spark.sql.catalyst.optimizer` pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
This pull request and its description were written by Isaac.
--
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]
