-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
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):
#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:
#19420 (comment)
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
PhysicalExprtrait /evaluatesignature:datafusion/physical-expr-common/src/physical_expr.rs- logical → physical expr conversion:
datafusion/physical-expr/src/planner.rs(Expr::BinaryExprmatch arm) - physical
BinaryExpr:datafusion/physical-expr/src/expressions/binary.rs ExecutionPropsandconfig_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:
- Add a new
ExecutionOptionsfield, e.g.binary_expr_preselection_threshold: f32(default0.2). - In
datafusion/physical-expr/src/planner.rswhen convertingExpr::BinaryExpr, read the option fromexecution_props.config_options(fallback to default whenNone), and set it on the constructed physicalBinaryExpr(e.g. viaBinaryExpr::with_preselection_threshold(threshold)or abinary_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 anyPhysicalExprthat needs it. - More invasive: change
PhysicalExpr::evaluateto accept aTaskContext(or similar) so evaluation can consult session options dynamically. This seems API-breaking and likely too broad for theBinaryExpruse case.
Expected outcome
- Users can configure
BinaryExprshort-circuit preselection viaSessionConfig/ SQLSET. EXPLAIN/ debug output reflects the configured threshold.- No API-breaking changes required for
PhysicalExpr::evaluate.