dwsmith1983 opened a new issue, #6971: URL: https://github.com/apache/datafusion-comet/issues/6971
### Describe the bug With ANSI mode on and subexpression elimination on (its default), Spark's projection evaluates a subexpression that several of its expressions share for every row, before any of their NULL checks. The JVM codegen dispatcher compiles each expression alone with Spark's own `doGenCode`, which skips an argument when an earlier one is NULL. So on a row whose first argument is NULL, a dispatched expression never evaluates the shared subexpression, and when that subexpression raises, Spark fails the query while Comet returns rows. Nothing on the dispatch path checks for this: `QueryPlanSerde.dispatchIfFallback`, `CometCodegenDispatch.convert` and the serdes that call the dispatcher directly (`replace`, `rlike`, the `regexp_*` functions) all emit the dispatch expression without looking at what the operator shares. Found in #5867 (https://github.com/apache/datafusion-comet/pull/5867#discussion_r4266282723), which adds this check for the six serdes that PR newly dispatches. This issue is about every other dispatch serde. ### Steps to reproduce On main (7d8d9b154), each of these fails as a SQL file test with "Expected Comet to throw an error matching 'DIVIDE_BY_ZERO' but query succeeded". Each table has a row whose first arguments are NULL and `x = 0.0D`. ```sql -- Config: spark.sql.ansi.enabled=true -- Config: spark.sql.subexpressionElimination.enabled=true -- a1, a2: array<double> SELECT array_contains(a1, 1.0D / x), array_contains(a2, 1.0D / x) FROM t; -- s1, s2: string SELECT replace(s1, CAST(1.0D / x AS STRING), 'z'), replace(s2, CAST(1.0D / x AS STRING), 'z') FROM t; SELECT rpad(s1, 6, CAST(1.0D / x AS STRING)), rpad(s2, 6, CAST(1.0D / x AS STRING)) FROM t; SELECT s1 LIKE CAST(1.0D / x AS STRING) ESCAPE '/', s2 LIKE CAST(1.0D / x AS STRING) ESCAPE '/' FROM t; -- k: int (NULL on that row) SELECT elt(k, s1, CAST(1.0D / x AS STRING)), elt(k, s2, CAST(1.0D / x AS STRING)) FROM t; -- Spark 4.x SELECT levenshtein(s1 COLLATE UTF8_LCASE, CAST(1.0D / x AS STRING)), levenshtein(s2 COLLATE UTF8_LCASE, CAST(1.0D / x AS STRING)) FROM t; ``` Reading the serdes, the same shape applies to most multi-argument dispatch routes: anything built on `nullSafeCodeGen` or `defineCodeGen` (for example `months_between`, `add_months`, `conv`, `hypot`, `find_in_set`, `format_number`, `overlay`, `mask`, `width_bucket`, the interval constructors), `StaticInvoke` and `Invoke` (which skip later arguments after a NULL one), and conditional routes such as `in`, `concat`, `lpad`, `sequence`, `trim` with a trim string, `array_intersect`, `array_except`, `translate` and the collated comparisons. A few nodes evaluate a later argument first: division and `pmod` skip the dividend when the divisor is NULL, `locate` skips its first two arguments when `start` is NULL, and `array_join` with a null replacement evaluates that first and skips the array. ### Expected behavior Comet raises `DIVIDE_BY_ZERO` like Spark. Once #5867 lands, its `QueryPlanSerde.skipsSharedSubexpression` check can move into `dispatchIfFallback` and the other dispatch entry points, so every dispatch serde falls back to Spark for these shapes, and the per-serde declines that PR adds go away. `array_join` needs the same evaluation-order exception as `locate`. ### Additional context The native `element_at` and `map_from_arrays` guards have a related gap, tracked in #6970. -- 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]
