[I] BETWEEN evaluates its operand two times, so a volatile operand returns wrong rows [datafusion]

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

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

   ### Describe the bug
   
   `x BETWEEN low AND high` evaluates `x` two times. When `x` is volatile, the two evaluations give two different values, and the query returns the wrong rows.
   
   ### To Reproduce
   
   ```sql
   CREATE TABLE v AS SELECT value AS a FROM generate_series(1, 100000);
   
   SELECT count(*) FROM v WHERE random() BETWEEN 0.4 AND 0.6;
   ```
   
   Result on `main` at 3a647e49dd:
   
   ```text
   +-------+
   | count |
   +-------+
   | 35751 |
   +-------+
   ```
   
   One draw of `random()` keeps the rows with `0.4 <= r <= 0.6`, which is about 20000 of 100000. The query keeps about 36000, which is `P(r1 >= 0.4) * P(r2 <= 0.6) = 0.6 * 0.6`. So each row draws two values.
   
   The plan shows the duplication:
   
   ```text
   > EXPLAIN SELECT count(*) FROM v WHERE random() BETWEEN 0.4 AND 0.6;
   logical_plan
   01)Projection: count(Int64(1)) AS count(*)
   02)--Aggregate: groupBy=[[]], aggr=[[count(Int64(1))]]
   03)----Filter: random() >= Float64(0.4) AND random() <= Float64(0.6)
   04)------TableScan: v projection=[]
   physical_plan
   05)--------FilterExec: random() >= 0.4 AND random() <= 0.6
   ```
   
   ### Expected behavior
   
   `random() BETWEEN 0.4 AND 0.6` keeps about 20000 of the 100000 rows. `x` is evaluated one time per row.
   
   ### Additional context
   
   There are two places that expand the operand:
   
   - `simplify_expressions` rewrites `Expr::Between` into `a >= low AND a <= high` and clones `between.expr`. See [`expr_simplifier.rs:1700`](https://github.com/apache/datafusion/blob/main/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs#L1700).
   - the physical planner does the same rewrite for any `Expr::Between` that reaches it. See [`planner.rs:446`](https://github.com/apache/datafusion/blob/main/datafusion/physical-expr/src/planner.rs#L446). It reuses one `Arc<dyn PhysicalExpr>` for both sides, but `evaluate` is still called two times, so a volatile function still draws two values.
   
   A fix has to cover both places. One option is to keep the operand in a single node, for example a dedicated `BetweenExpr` physical expression, or to let the two comparisons share one evaluated array.
   
   The same duplication makes an expensive operand run two times. `SELECT abs(c1) BETWEEN 0 AND log(c1 * 100) FROM t` calls `abs` two times per row. That is a cost problem, not a correctness problem.
   
   This was found with a plan invariant that counts the evaluation sites of volatile and `KeepInPlace` calls before and after each optimizer rule. Related guards against the same class: https://github.com/apache/datafusion/issues/24678, https://github.com/apache/datafusion/issues/25415.
   


-- 
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

Re: [I] BETWEEN evaluates its operand two times, so a volatile operand returns wrong rows [datafusion]

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

buraksenn commented on issue #25457:
URL: https://github.com/apache/datafusion/issues/25457#issuecomment-5728151513

   take


-- 
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

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