andygrove commented on PR #6716: URL: https://github.com/apache/datafusion-comet/pull/6716#issuecomment-6110180024
Thanks, you were right that the eager path loses here. I measured four shapes with ANSI off, before dd6da4237d and at it, on a batch of 8,192 rows with no NULLs. Each figure is the median of two runs, in µs per batch. | Shape | Before | dd6da4237d | | -------------------------------------------------------- | -----: | ---------: | | `IF(c2 < 5, CAST(c8 AS INT), NULL)` | 9.3 | 75 | | `CASE` with three branches, each parsing for 2% of rows | 76 | 229 | | `IF(c2 < 95, CAST(c8 AS INT), NULL)` | 89 | 76 | | `IF(c2 < 50, CAST(c8 AS INT), CAST(c9 AS INT))` | 206 | 152 | Parsing a whole batch costs about 75 µs, whichever rows the branch selects. The eager path saves up to a quarter when most rows parse, but it makes the selective `IF` 8x slower and the three-branch `CASE` 3x slower. `CaseWhenExpr` makes the choice once per expression, before it sees which rows match, so the lazy path is the safer one for these casts. 22d41596da splits the check as you suggested. `Cast::is_infallible` is back to the casts that cannot fail and cost little more than reading their input, and `Cast::cannot_fail` adds the LEGACY and TRY string-to-integer casts. In `case_when.rs`, `CASE` uses `is_infallible` and `NullShortCircuit` uses `cannot_fail`, since it masks an argument only to keep it from failing on rows that Spark skips. So `CASE` chooses the path it chose before dd6da4237d for every expression, and `array_contains(ids, CAST(s AS INT))` still plans unwrapped with ANSI off. The doc comments on both checks now say which casts they accept and why. `infallible_expressions` checks that `cannot_fail` accepts every LEGACY and TRY string-to-integer cast and `is_infallible` doesn't, and `infallible_arguments_are_not_wrapped` covers a LEGACY cast as a later argument. The four shapes are now in `benches/conditional.rs`. On Spark 4.1, the `array/`, `map/` and `conditional/` fixtures and the two expression suites pass (241 tests). The `spark-expr` tests pass, and clippy is clean on stable and nightly. -- 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]
