dwsmith1983 opened a new issue, #6972: URL: https://github.com/apache/datafusion-comet/issues/6972
### Describe the bug `QueryPlanSerde.canShortCircuitNulls` keeps every argument of an expression eager when a later argument holds a subexpression that the operator's expressions share, so that an error Spark raises from the hoisted subexpression is raised in Comet too. It treats them as shared whatever `spark.sql.subexpressionElimination.enabled` says. With subexpression elimination off, Spark does not hoist anything, and its own null short-circuit skips the later argument on a row whose first argument is NULL. Comet evaluates it eagerly on that row, so it raises where Spark returns rows. Found while working on #6970 (Found in #5867, https://github.com/apache/datafusion-comet/pull/5867#discussion_r4266282723). ### Steps to reproduce On main (7d8d9b154), as a SQL file test: ```sql -- Config: spark.sql.ansi.enabled=true -- Config: spark.sql.subexpressionElimination.enabled=false statement CREATE TABLE t(m1 map<int, int>, m2 map<int, int>, i int) USING parquet statement INSERT INTO t VALUES (CAST(NULL AS MAP<INT, INT>), CAST(NULL AS MAP<INT, INT>), 0), (map(1, 5), map(1, 6), 1) query SELECT m1[1 div i], m2[1 div i] FROM t ``` Spark returns `NULL, NULL` and `5, 6`. Comet runs the projection natively and fails with `DIVIDE_BY_ZERO`. ### Expected behavior The same rows as Spark. `canShortCircuitNulls` can return true when `spark.sql.subexpressionElimination.enabled` is off, since Spark's projection and aggregate only hoist shared subexpressions when it is on. Every caller of `withNullShortCircuit` (the map lookup and the array functions that use it) picks that up. ### Additional context With the setting on, the same function is also broader than Spark in one way: a later argument counts as shared when any piece of it is shared, and the whole argument is then evaluated for every row, while Spark hoists only the shared piece. For example `map_from_arrays(CASE WHEN k = 0 THEN NULL ELSE array(1) END, CASE WHEN k = 0 THEN array(1 / k, 2) ELSE array(1 / k) END)` shares only `k = 0`, which cannot fail. Exact handling would need native support for evaluating only the shared pieces, so that part is noted here rather than proposed. -- 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]
