Unify the `ExecBuilder` pattern across physical operators
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Is your feature request related to a problem or challenge?
We've begun adding builders to physical operators so that new fields can be added without breaking `try_new`/`new` signatures. Four exist today:
- `FilterExecBuilder` (`datafusion/physical-plan/src/filter.rs:105`)
- `HashJoinExecBuilder` (`datafusion/physical-plan/src/joins/hash_join/exec.rs:288`)
- `NestedLoopJoinExecBuilder` (`datafusion/physical-plan/src/joins/nested_loop_join.rs:232`)
- `AnalyzeExecBuilder` (`datafusion/physical-plan/src/analyze.rs:72`)
They were added incrementally and have diverged in ways that will be baked in if we roll the pattern out to the remaining ~30 operators:
| | `FilterExecBuilder` | `HashJoinExecBuilder` | `NestedLoopJoinExecBuilder` | `AnalyzeExecBuilder` |
|---|---|---|---|---|
| Internal repr | mirrored fields | wrapped `exec` + `preserve_properties` | mirrored fields | mirrored fields |
| `build()` returns | `Result` | `Result` (+ `build_exec()` -> `Result>`) | `Result` | `X`, infallible |
| `From<&X>` | yes (`filter.rs:225`) | yes (`exec.rs:520`) | yes (`nested_loop_join.rs:312`) | no |
| `X::builder()` | no | yes, `(&self) -> Builder` (`exec.rs:852`) | no | yes, `(verbose, show_statistics, input, schema)` (`analyze.rs:136`) |
| Projection setter | `apply_projection` -> `Result`, **composes** with existing | `with_projection`, infallible, **replaces** | `with_projection`, infallible, **replaces** | n/a |
Two of these are genuine footguns rather than cosmetic drift:
- **`X::builder()` means opposite things.** `HashJoinExec::builder(&self)` clones an existing plan into a builder; `AnalyzeExec::builder(a, b, c, d)` is an associated constructor taking required args. Same name, incompatible signatures.
- **Projection setters differ in semantics, not just name.** `FilterExecBuilder::apply_projection` composes through an existing projection (current `[0, 2, 3]` + `[0, 2]` -> `[0, 3]`); the join builders' `with_projection` overwrites. Anyone porting code between operators gets wrong results, not a compile error.
### Describe the solution you'd like
Agree on one canonical shape, document it in `docs/source/contributor-guide/api-health.md` next to the deprecation guidelines (builders are currently undocumented there), then align the four existing builders before extending the pattern further.
Points to settle:
1. **Internal representation.** Mirrored fields, or `HashJoinExecBuilder`'s wrapped-exec + `preserve_properties` flag? The wrapped approach avoids duplicating the field list (adding a field = one setter, not three edits) and recomputes `PlanProperties` once in `build()` rather than per-setter — see #20276. It does require every setter author to remember to invalidate the flag.
2. **`build()` fallibility.** Uniformly `Result`, even where currently infallible, so a validation can be added later without a breaking change? Also whether `build_exec() -> Result>` should exist on all builders or none.
3. **Getting a builder from an existing plan.** Standardize on `From<&X>`, and either drop `X::builder()` or fix its meaning to one of the two.
4. **Projection semantics.** Pick compose or replace, and name the two behaviors distinctly if both are genuinely needed.
5. **Should required constructor args also get setters?** `FilterExecBuilder` has `with_input`/`with_predicate`; the join builders don't.
### Describe alternatives you've considered
Keep `new(required) + with_*` on the exec itself. Adding a private field plus a `with_*` setter is already non-breaking, so this partly works — but it recomputes `PlanProperties` on every setter call (e.g. `SortExec::with_fetch`, `sort.rs:1057`) and can't validate field combinations, which is what motivated builders in the first place.
Leave the existing four alone and only apply a convention to new builders — cheaper now, but leaves the inconsistency permanently in the public API.
### Additional context
Related: #20276, #19893, #23708.
Follow-on work (builders for the remaining operators — `SymmetricHashJoinExec` and `HashJoinExec` take 9 constructor args, `SortMergeJoinExec` and `PiecewiseMergeJoinExec` 7, `AggregateExec` and `StreamingTableExec` 6) should be a separate tracking issue once the shape is settled.
Contributor guide
Assessment
This issue has not been assessed yet.