apache / apache/datafusion

[Proposal] Propagate ExecutionOptions into Physical BinaryExpr

Open
#19,709 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

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):
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`). 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` during physical expr planning).

### Alternatives / open questions

- More general solution: introduce a lightweight “physical expression config” object (or pass `Arc`) 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`.

Contributor guide

Open the contributing guide

Research direction

Start by reading ExecutionProps in datafusion/expr/src/execution_props.rs, then trace the Expr::BinaryExpr conversion in datafusion/physical-expr/src/planner.rs into datafusion/physical-expr/src/expressions/binary.rs. Done means the configured threshold is captured at plan time, defaults correctly, is exposed through SessionConfig or SQL SET, and appears in EXPLAIN or debug output without changing PhysicalExpr::evaluate.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, backend-api-design
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.