andygrove opened a new pull request, #7326: URL: https://github.com/apache/datafusion-comet/pull/7326
## Which issue does this PR close? Closes #6823. Part of #6006. ## Rationale for this change Spark evaluates `AND`, `OR` and the candidates of `IN` lazily: a later operand runs only for the rows that the earlier ones have not decided. Comet evaluates every operand over the whole batch, so under ANSI mode an operand that can raise throws on a row Spark never evaluates. With `v` holding 0 to 3 on a Parquet table and ANSI on, each of these raised `DIVIDE_BY_ZERO` in Comet and returned rows in Spark: - `v != 0 AND 100 / v > 1` - `v = 0 OR 100 / v > 1` - `v IN (0, 100 / v)` `CASE WHEN`, `IF` and `coalesce` already match Spark. ## What changes are included in this PR? - `LazyLogical` (`native/spark-expr/src/conditional_funcs/lazy_logical.rs`) evaluates the right operand of `AND` and `OR` only for the rows the left operand has not decided, which are the rows where it is not false for `AND` and not true for `OR`. It reuses the column projection that `NullShortCircuit` uses, so skipping rows filters no other column. The `AND` and `OR` builders now create it. - It applies only when the right operand can fail, by the existing `cannot_fail` check. Otherwise the expression stays a plain `BinaryExpr`, so the common case has no extra cost. - `lazy_in_list` rewrites `v IN (c1, c2, ...)` into a chain of lazy equalities when a candidate after the first can fail. `NOT IN` is the negation of that chain. This applies only when the value is a cheap expression that cannot fail, because the chain evaluates it once per candidate. Other shapes keep the existing whole-batch evaluation. This does not cover the rest of #6006: aggregate `FILTER`, operator-level masks, and the compatibility guide section. ## How are these changes tested? - Rust unit tests in `lazy_logical.rs` cover `AND` and `OR` on rows the left side decides, a NULL left operand, a batch with no decided rows, an empty batch, a scalar left operand, an infallible right operand staying a `BinaryExpr`, and the `IN` and `NOT IN` rewrite. The `datafusion-comet-spark-expr` lib suite passes (1330 tests) and `cargo clippy -D warnings` is clean. - New SQL file test `expressions/conditional/logical_lazy_ansi.sql` runs the cases above, chains, nesting under `NOT`, and `IN` with a failing cast. It also has controls where a row Spark does evaluate must still raise `DIVIDE_BY_ZERO` or `CAST_INVALID_INPUT`. It passes with `CometSqlFileTestSuite` on Spark 4.1, along with the existing `conditional/` and `predicates` SQL files. - I did not run the Spark SQL suite or the other Spark profiles. -- 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]
