timsaucer commented on code in PR #13823:
URL: https://github.com/apache/datafusion/pull/13823#discussion_r1890183926
##########
datafusion/ffi/src/plan_properties.rs:
##########
@@ -220,50 +235,89 @@ impl TryFrom<FFI_PlanProperties> for PlanProperties {
RErr(e) => Err(DataFusionError::Plan(e.to_string())),
}?;
- let execution_mode: ExecutionMode =
- unsafe { (ffi_props.execution_mode)(&ffi_props).into() };
-
let eq_properties = match orderings {
Some(ordering) => {
EquivalenceProperties::new_with_orderings(Arc::new(schema),
&[ordering])
}
None => EquivalenceProperties::new(Arc::new(schema)),
};
+ let emission_type: EmissionType =
+ unsafe { (ffi_props.emission_type)(&ffi_props).into() };
+
+ let boundedness: Boundedness =
+ unsafe { (ffi_props.boundedness)(&ffi_props).into() };
+
Ok(PlanProperties::new(
eq_properties,
partitioning,
- execution_mode,
+ emission_type,
+ boundedness,
))
}
}
-/// FFI safe version of [`ExecutionMode`].
+/// FFI safe version of [`Boundedness`].
#[repr(C)]
#[allow(non_camel_case_types)]
#[derive(Clone, StableAbi)]
-pub enum FFI_ExecutionMode {
+pub enum FFI_Boundedness {
Bounded,
- Unbounded,
Review Comment:
Yes, the idea is to be stable. I don't think the real problem here is the
change from `FFI_ExecutionMode` to `FFI_Boundness` but rather the way it breaks
libraries working across the boundary that will expect one and receive the
other. I think this current impact is minimal since the FFI interfaces have
*just* been released and I don't think many people have had an opportunity to
use them. However I will open an issue about adding a feature to validate the
versioning across the boundary. I think that will be necessary going forward.
--
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]
