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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to