tlfua opened a new issue, #7587: URL: https://github.com/apache/datafusion-comet/issues/7587
### What is the problem the feature request solves? https://github.com/apache/datafusion-comet/pull/7456 evaluates an operand of an ANSI arithmetic expression or `concat` only for the rows that Spark evaluates it for, when the operand could fail on the rows Spark skips. To decide whether to evaluate such an operand over the whole batch anyway and fall back to the selected rows only if that fails, `NullShortCircuit` asks `is_cheap` (`native/spark-expr/src/conditional_funcs/null_short_circuit.rs`) whether the operand costs about as little over the whole batch as over the selected rows. `is_cheap` only accepts columns, literals, integer and floating point `+`, `-`, `*` (including `checked_add`, `checked_sub` and `checked_mul`) and widening casts. `cannot_fail` does not know any function, so in `concat(q, upper(s))` the `upper(s)` argument is masked, and since `is_cheap` treats `upper` as costly it is evaluated over the whole batch only when at least half the rows need it, and on the selected rows otherwise, even where speculating would be faster. The `concat_upper` shape in `native/spark-expr/benches/null_short_circuit.rs` measures this case. Raised in review: https://github.com/apache/datafusion-comet/pull/7456#pullrequestreview-5831312396 ### Describe the potential solution Benchmark `concat_upper` and similar string shapes (`lower`, `trim`, `substring`) with every row selected, a few rows selected, and in between, comparing speculating over the whole batch against evaluating only the selected rows. Add to `is_cheap` the functions for which speculating is never much slower, with the benchmark numbers in the PR, as the doc comment of `is_cheap` asks ("The list holds what has been measured, and can grow"). ### Additional context `is_cheap` only affects performance. Whether an operand is masked at all is decided by `cannot_fail` in `native/spark-expr/src/conditional_funcs/case_when.rs`, so a change to `is_cheap` cannot change results. ### Willingness to contribute I can contribute a fix for this bug independently -- 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]
