apache / apache/datafusion

FFI_PlanProperties drops scheduling_type and evaluation_type, so every plan crossing FFI misreports both

Open
#25,153 0 comments 0 reactions 0 assignees View on GitHub
bug ffi
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Describe the bug

`FFI_PlanProperties` (`datafusion/ffi/src/plan_properties.rs:38-66`) exposes accessors for `output_partitioning`, `emission_type`, `boundedness`, `output_ordering`, and `schema` — but not for `scheduling_type` or `evaluation_type`. Neither identifier appears anywhere in that file.

Reconstruction on the receiving side goes through `PlanProperties::new` (`plan_properties.rs:189`, `:280`, `:299`), which initialises both fields to their defaults:

```rust
// datafusion/physical-plan/src/execution_plan.rs:1521-1522
evaluation_type: EvaluationType::Lazy,
scheduling_type: SchedulingType::NonCooperative,
```

So **every** node that crosses an FFI boundary reports `NonCooperative` and `Lazy` regardless of what it actually is.

The visible consequence is in `EnsureCooperative`, which is the only rule in `datafusion/physical-optimizer/src/` that reads either field. It wraps non-cooperative leaves in a `CooperativeExec`. Given a foreign leaf that is genuinely `Cooperative`, it sees `NonCooperative` and wraps it anyway — adding a redundant operator to the plan.

`EvaluationType` is also load-bearing in that rule: its ancestry walk treats an `Eager` ancestor as resetting the cooperative context (`datafusion/physical-optimizer/src/ensure_coop.rs:100-103`). With every foreign node reporting `Lazy`, that reset never triggers across a boundary.

### To Reproduce

Any plan crossing FFI shows it. From a reproduction of the related planner-boundary issue, the same tree printed at two levels:

```text
ForeignExecutionPlan { name: "CooperativeExec",
properties: PlanProperties { ..., evaluation_type: Lazy,
scheduling_type: NonCooperative },
children: [ EmptyExec { ...,
cache: PlanProperties { ..., scheduling_type: Cooperative } } ] }
```

The `EmptyExec` is `Cooperative`. The foreign view of it is not — which is why the `CooperativeExec` above it was inserted at all.

### Expected behavior

`PlanProperties` should round-trip through FFI without losing fields. A foreign node should report the same `scheduling_type` and `evaluation_type` as the node it wraps.

### Additional context

The fix looks mechanical and follows patterns already in the file:

- `FFI_EmissionType` and `FFI_Boundedness` (`plan_properties.rs:201-260`) are stabby enums with bidirectional `From` impls — the same shape works for `SchedulingType` and `EvaluationType`.
- Two more fn-pointer fields on `FFI_PlanProperties`, matching the existing `emission_type` / `boundedness` entries.
- `PlanProperties::with_scheduling_type` and `with_evaluation_type` already exist (`datafusion/physical-plan/src/execution_plan.rs:1562,1570`) and can be chained onto the three reconstruction sites.

Adding fields to a `#[repr(C)]` struct is an ABI change, but `datafusion-ffi` gates compatibility on the crate major version (`datafusion/ffi/src/lib.rs:64`), which bumps with each DataFusion major release.

This defect is the trigger for the most common symptom in the FFI query planner issue — the spurious `CooperativeExec` it produces is precisely the node that then fails to serialize. Fixing this does not fix that issue's general class, only its most frequent instance. See umbrella #25152 for the relationship.

**Relationship to #22329.** That issue covers the same family — FFI silently dropping optimizer-relevant information — but for a different struct: it lists defaulted *methods* missing from `FFI_ExecutionPlan`, whereas this is two missing *fields* on `FFI_PlanProperties`. Neither `scheduling_type` nor `evaluation_type` appears in its list. Worth noting while there: two entries in #22329 have since landed — `apply_expressions` and `partition_statistics` are both in the `FFI_ExecutionPlan` vtable today — so its list needs a refresh.

Contributor guide

Open the contributing guide

Research direction

Start in datafusion/ffi/src/plan_properties.rs:38-66 and read the existing FFI_EmissionType and FFI_Boundedness conversions. Check the reconstruction sites in datafusion/physical-plan/src/execution_plan.rs and the consumers in datafusion/physical-optimizer/src/ensure_coop.rs. Done means scheduling_type and evaluation_type survive an FFI round trip and EnsureCooperative no longer adds a redundant wrapper for a cooperative foreign node.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, data-engineering
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.