timsaucer opened a new issue, #25153:
URL: https://github.com/apache/datafusion/issues/25153
### 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.
--
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]
