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: [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]

Reply via email to