tlfua commented on PR #7456: URL: https://github.com/apache/datafusion-comet/pull/7456#issuecomment-6695025317
Thanks @coderfender, all four are addressed in 9b6c7be04. The full table is in the updated description ([bench run](https://github.com/tlfua/datafusion-comet/actions/runs/42270828909)). 1. Done as you suggested. `is_cheap` in `null_short_circuit.rs` lists columns, literals, integer and floating point `+`, `-` and `*` (including `checked_add`, `checked_sub` and `checked_mul`) and widening casts, over cheap children only. Such an argument is always speculated first. Anything else, such as a parsing `Cast`, decimal arithmetic or a string function, keeps the half-NULL threshold. `a * (b - c)` under ANSI is now within 5% of eager at every NULL ratio (11.0 vs 10.6 µs at 70% NULL, where it was 14.0). I also benchmarked the borderline candidates. Always speculating a `WideDecimalBinaryExpr` multiply or a rescaling decimal cast is 2.1x slower than selecting rows at 70% NULL, and the decimal `p * CheckOverflow(1.00 - d)` 1.2x to 1.9x at 70 to 90%, so they stay off the list. `concat(q, upper(s))` is the one shape where this falls short: with `upper(s)` kept off the list, as you suggested for string functions, it is 1.4x slower than eager at 50% NULL and 1.2x at 70%. Always speculating it would bring that to 1.09x and 1.04x, and cost 5% at 90%. So infallible string functions look worth adding to the list, but each needs measuring. I'd prefer a follow-up, but can add `upper` here if you'd rather. 2. The table now has `nulls_0`, and four new shapes: LEGACY `p * CheckOverflow(1.00 - d)` (your Q1 case), `concat(q, upper(s))`, a wide decimal multiply and a rescaling decimal cast. With no NULLs, the wrapper is within 3% of eager for every shape (`a * (b - c)` is 14.8 vs 14.4 µs, LEGACY Q1 42.1 vs 41.8). 3. Added `try_divide(CAST(txt AS INT), n) FROM zero_bad` and a decimal variant, both NULL as in Spark. The new `mostly_null` table has `n` NULL on three rows in four. `n * (big - 1)` is cheap, so it is speculated, overflows on a skipped row and falls back to selecting rows. `n + CAST(txt AS INT)`, `CAST(txt AS INT) DIV n` and the two `concat` queries select the rows without speculating. 4. Filed https://github.com/apache/datafusion-comet/issues/7542 to track the `branch-1.2` backport. -- 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]
