andygrove commented on PR #6397: URL: https://github.com/apache/datafusion-comet/pull/6397#issuecomment-5892376529
This overlaps with #6350, which is approved and only needs a rebase. It replaces the `CaseExpr` inside `IfExpr` with a new `CaseWhenExpr` that evaluates every branch over the whole batch when none of them can fail. Columns and literals always qualify, so `IF(isnan(c), 0D, c)` already avoids the filter and merge there, using the same approach as `select_primitive`: start from the column's values and write the literal into the rows that choose it. It also covers strings, booleans, computed branches that cannot fail, `CASE WHEN` and nested `IF`s. The two PRs conflict in `if_expr.rs`. If both merged, the column/literal path here would run in front of `CaseWhenExpr` and do largely the same work. Could you try your #6180 workload on top of #6350? If the gap is gone, I'd suggest cutting this PR down to the new tests in `if_expr.sql`, which are worth having either way. If there's still a gap, or a type that `CaseWhenExpr` still hands to `CaseExpr` matters for you, I'd rather fix it in `CaseWhenExpr` so that `CASE WHEN` benefits too, with the benchmark cases added to `benches/conditional.rs`. -- 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]
