[I] Wrong results: COALESCE on a volatile operand evaluates it two times, and fails when the output is declared non-nullable [datafusion]

["adriangb (via GitHub)" <gi...@apache.org>]

adriangb opened a new issue, #25477:
URL: https://github.com/apache/datafusion/issues/25477

   ### Describe the bug
   
   `COALESCE` on a volatile operand evaluates the operand two times. The null test and the returned value are two different draws of `random()`. When the last argument is a non-null literal, the planner declares the output column non-nullable, and the query fails at run time with an Arrow error. With a nullable last argument the query completes and returns rows where `COALESCE` returned `NULL`, which is not possible for a single evaluation.
   
   The cause is the rewrite in `datafusion/functions/src/core/coalesce.rs`: `simplify` turns `coalesce(a, b)` into `CASE WHEN a IS NOT NULL THEN a ELSE b END`, which names `a` two times. The rewrite cannot be skipped for a volatile `a`, because `coalesce` has no runtime kernel: `invoke_with_args` returns an internal error that says the function must be simplified first.
   
   This is the same mechanism as https://github.com/apache/datafusion/issues/25457 (`BETWEEN`), which https://github.com/apache/datafusion/pull/25476 fixes with a physical expression that evaluates the operand one time. `COALESCE` needs its own fix, either a runtime kernel for the volatile case or a rewrite that binds `a` one time.
   
   ### To Reproduce
   
   `main` at 0e292dcbfd, and the leaf-pushdown integration branch, with `datafusion-cli`:
   
   ```sql
   CREATE TABLE v AS SELECT value AS a FROM generate_series(1, 100000);
   SELECT count(*), count(c) FROM (SELECT coalesce(nullif(floor(random() * 2), 0), -1) AS c FROM v);
   ```
   
   ```text
   Error: Arrow error: Invalid argument error: Column 'c' is declared as non-nullable but contains null values
   ```
   
   `EXPLAIN` shows the two evaluations:
   
   ```text
   ProjectionExec: expr=[CASE WHEN nullif(floor(random() * 2), 0) IS NOT NULL THEN nullif(floor(random() * 2), 0) ELSE -1 END as c]
   ```
   
   ### Expected behavior
   
   The query completes. `c` is `1` for about half of the rows and `-1` for the other half, and it is never `NULL`, because `random()` is evaluated one time per row.
   
   DuckDB 1.5.2 is not a reference for this shape: it also re-evaluates the volatile operand, and its counts for the same query do not add up to the row count.
   
   ### Additional context
   
   Found while fixing https://github.com/apache/datafusion/issues/25457. The evaluation-site invariant in https://github.com/apache/datafusion/pull/25458 reports this shape.
   
   Tracked in the leaf-pushdown EPIC: https://github.com/apache/datafusion/issues/25459
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org