LuciferYang commented on PR #59186: URL: https://github.com/apache/spark/pull/59186#issuecomment-5936869552
Closing this. The premise was wrong: this is not a bug fix. A `With` has been evaluable since SPARK-58818, so an inferred filter that carries one already returns the correct rows. It evaluates the definition once per row rather than once per reference. Inlining trades that for visibility to `ReplaceNullWithFalseInPredicate` and to V2 predicate translation, and in the common case the trade loses. The only `With` that reaches this rule is one `RewriteWithExpression` kept because its definition is referenced more than once and is not cheap, so inlining evaluates an expensive definition repeatedly. File sources gain nothing, since V1 translation has no case for the surrounding `CaseWhen` either way, and V2 cannot translate a UDF definition. On `CASE WHEN a > 0 THEN udf(a) BETWEEN 1 AND 100 ELSE false END` under an equi-join, the UDF calls in the inferred filter went from 18 to 27, with identical results. Gating the inlining on cheapness would not help: by construction, every `With` that reaches this rule fails that test. I'll resolve SPARK-59922 as Won't Fix. Thanks @HyukjinKwon for the review. -- 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]
