acking-you opened a new issue, #19709: URL: https://github.com/apache/datafusion/issues/19709
## Proposed Title Propagate `ExecutionOptions` (ConfigOptions) into physical `BinaryExpr` / `PhysicalExpr` evaluation (needed for configurable `preselection_threshold`) ## Description (paste into GitHub issue) ### Background PR #19420 adds a `preselection_threshold` knob to `BinaryExpr` to improve short-circuit evaluation for `AND` when the RHS is expensive (or may error if evaluated unnecessarily): https://github.com/apache/datafusion/pull/19420 In the PR discussion, it was noted that there is currently **no way to read `ExecutionOptions` inside `PhysicalExpr::evaluate`**, which blocks wiring this knob to a user-facing/session-level configuration: https://github.com/apache/datafusion/pull/19420#issuecomment-3724107305 ### Problem `PhysicalExpr::evaluate(&RecordBatch)` does not take any session/task context, so built-in physical expressions (such as `BinaryExpr`) cannot consult `ExecutionOptions` at evaluation time. While `datafusion_physical_expr::create_physical_expr` receives `ExecutionProps` (which can carry a snapshot of `ConfigOptions`/`ExecutionOptions`), today that snapshot is only used for some expressions (for example, `ScalarFunctionExpr` captures `Arc<ConfigOptions>`). The `Expr::BinaryExpr` conversion path does not propagate any `ExecutionOptions`-derived settings into the created physical `BinaryExpr`, so the new behavior can’t be configured via `ExecutionOptions`. **Relevant code pointers** - `PhysicalExpr` trait / `evaluate` signature: `datafusion/physical-expr-common/src/physical_expr.rs` - logical → physical expr conversion: `datafusion/physical-expr/src/planner.rs` (`Expr::BinaryExpr` match arm) - physical `BinaryExpr`: `datafusion/physical-expr/src/expressions/binary.rs` - `ExecutionProps` and `config_options`: `datafusion/expr/src/execution_props.rs` ### Proposal (minimal / non-breaking) Capture the needed option at **plan time**, and store it in the physical `BinaryExpr`, rather than trying to read session config during `evaluate`. Concretely: 1. Add a new `ExecutionOptions` field, e.g. `binary_expr_preselection_threshold: f32` (default `0.2`). 2. In `datafusion/physical-expr/src/planner.rs` when converting `Expr::BinaryExpr`, read the option from `execution_props.config_options` (fallback to default when `None`), and set it on the constructed physical `BinaryExpr` (e.g. via `BinaryExpr::with_preselection_threshold(threshold)` or a `binary_with_options(...)` constructor). This matches the existing pattern used by `ScalarFunctionExpr` (which captures `Arc<ConfigOptions>` during physical expr planning). ### Alternatives / open questions - More general solution: introduce a lightweight “physical expression config” object (or pass `Arc<ConfigOptions>`) that can be captured by any `PhysicalExpr` that needs it. - More invasive: change `PhysicalExpr::evaluate` to accept a `TaskContext` (or similar) so evaluation can consult session options dynamically. This seems API-breaking and likely too broad for the `BinaryExpr` use case. ### Expected outcome - Users can configure `BinaryExpr` short-circuit preselection via `SessionConfig` / SQL `SET`. - `EXPLAIN` / debug output reflects the configured threshold. - No API-breaking changes required for `PhysicalExpr::evaluate`. -- 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]
