rebase_main

Author: coderfender

datafusion/ffi/README.md

@@ -91,8 +91,8 @@ challenges for our use case:
    function pointers that reference `&Self`, which complicates `IStable`
    implementations.
 
-3. **FFI_Option and FFI_Result**: For similar reasons, we provide our own
-   `FFI_Option<T>` and `FFI_Result<T, E>` types using `#[repr(C, u8)]` instead
+3. **FFI_Option and FFIResult**: For similar reasons, we provide our own
+   `FFI_Option<T>` and `FFIResult<T>` types using `#[repr(C, u8)]` instead
    of stabby's `Option` and `Result`, which require inner types to be `IStable`.
 
 This hybrid approach gives us stabby's maintained, ABI-stable collection types

datafusion/ffi/src/catalog_provider.rs

@@ -30,7 +30,7 @@ use tokio::runtime::Handle;
 use crate::execution::FFI_TaskContextProvider;
 use crate::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
 use crate::schema_provider::{FFI_SchemaProvider, ForeignSchemaProvider};
-use crate::util::{FFI_Option, FFI_Result, FFIResult};
+use crate::util::{FFI_Option, FFIResult};
 use crate::{df_result, sresult_return};
 
 /// A stable struct for sharing [`CatalogProvider`] across FFI boundaries.
@@ -152,7 +152,7 @@ unsafe extern "C" fn register_schema_fn_wrapper(
                 })
                 .into();
 
-        FFI_Result::Ok(returned_schema)
+        FFIResult::Ok(returned_schema)
     }
 }
 
@@ -168,7 +168,7 @@ unsafe extern "C" fn deregister_schema_fn_wrapper(
         let maybe_schema =
             sresult_return!(inner_provider.deregister_schema(name.as_str(), cascade));
 
-        FFI_Result::Ok(
+        FFIResult::Ok(
             maybe_schema
                 .map(|schema| {
                     FFI_SchemaProvider::new_with_ffi_codec(

datafusion/ffi/src/config/extension_options.rs

@@ -27,7 +27,7 @@ use stabby::string::String as SString;
 use stabby::vec::Vec as SVec;
 
 use crate::df_result;
-use crate::util::{FFI_Result, FFIResult};
+use crate::util::{FFIResult};
 
 /// A stable struct for sharing [`ExtensionOptions`] across FFI boundaries.
 ///
@@ -99,7 +99,7 @@ unsafe extern "C" fn set_fn_wrapper(
     let _ = options
         .inner_mut()
         .insert(key.as_str().into(), value.as_str().into());
-    FFI_Result::Ok(())
+    FFIResult::Ok(())
 }
 
 unsafe extern "C" fn entries_fn_wrapper(

datafusion/ffi/src/execution_plan.rs

@@ -137,7 +137,7 @@ unsafe extern "C" fn with_new_children_fn_wrapper(
     let children = sresult_return!(children);
     let new_plan = sresult_return!(inner_plan.with_new_children(children));
 
-    crate::ffi_option::FFI_Result::Ok(FFI_ExecutionPlan::new(new_plan, runtime))
+    FFIResult::Ok(FFI_ExecutionPlan::new(new_plan, runtime))
 }
 
 unsafe extern "C" fn execute_fn_wrapper(

datafusion/ffi/src/ffi_option.rs

@@ -22,6 +22,8 @@
 //! pointers and cannot implement `IStable`. These simple `#[repr(C)]` types
 //! provide the same FFI-safe semantics without that constraint.
 
+use stabby::string::String as SString;
+
 /// An FFI-safe option type.
 #[repr(C, u8)]
 #[derive(Debug, Clone)]
@@ -68,65 +70,65 @@ impl<T> FFI_Option<T> {
     }
 }
 
-/// An FFI-safe result type.
+/// An FFI-safe result type with SString as the error type.
 #[repr(C, u8)]
 #[derive(Debug, Clone)]
-pub enum FFI_Result<T, E> {
+pub enum FFIResult<T> {
     Ok(T),
-    Err(E),
-}
-
-impl<T, E> From<Result<T, E>> for FFI_Result<T, E> {
-    fn from(res: Result<T, E>) -> Self {
-        match res {
-            Ok(v) => FFI_Result::Ok(v),
-            Err(e) => FFI_Result::Err(e),
-        }
-    }
-}
-
-impl<T, E> From<FFI_Result<T, E>> for Result<T, E> {
-    fn from(res: FFI_Result<T, E>) -> Self {
-        match res {
-            FFI_Result::Ok(v) => Ok(v),
-            FFI_Result::Err(e) => Err(e),
-        }
-    }
+    Err(SString),
 }
 
-impl<T, E> FFI_Result<T, E> {
+impl<T> FFIResult<T> {
     pub fn is_ok(&self) -> bool {
-        matches!(self, FFI_Result::Ok(_))
+        matches!(self, FFIResult::Ok(_))
     }
 
     pub fn is_err(&self) -> bool {
-        matches!(self, FFI_Result::Err(_))
+        matches!(self, FFIResult::Err(_))
     }
 
-    pub fn unwrap_err(self) -> E {
+    pub fn unwrap_err(self) -> SString {
         match self {
-            FFI_Result::Err(e) => e,
-            FFI_Result::Ok(_) => panic!("called unwrap_err on Ok"),
+            FFIResult::Err(e) => e,
+            FFIResult::Ok(_) => panic!("called unwrap_err on Ok"),
         }
     }
 
-    pub fn map<U, F: FnOnce(T) -> U>(self, f: F) -> FFI_Result<U, E> {
+    pub fn map<U, F: FnOnce(T) -> U>(self, f: F) -> FFIResult<U> {
         match self {
-            FFI_Result::Ok(v) => FFI_Result::Ok(f(v)),
-            FFI_Result::Err(e) => FFI_Result::Err(e),
+            FFIResult::Ok(v) => FFIResult::Ok(f(v)),
+            FFIResult::Err(e) => FFIResult::Err(e),
         }
     }
 
-    pub fn into_result(self) -> Result<T, E> {
+    pub fn into_result(self) -> Result<T, SString> {
         self.into()
     }
 }
 
-impl<T: PartialEq, E: PartialEq> PartialEq for FFI_Result<T, E> {
+impl<T> From<FFIResult<T>> for Result<T, SString> {
+    fn from(res: FFIResult<T>) -> Self {
+        match res {
+            FFIResult::Ok(v) => Ok(v),
+            FFIResult::Err(e) => Err(e),
+        }
+    }
+}
+
+impl<T, E: ToString> From<Result<T, E>> for FFIResult<T> {
+    fn from(res: Result<T, E>) -> Self {
+        match res {
+            Ok(v) => FFIResult::Ok(v),
+            Err(e) => FFIResult::Err(SString::from(e.to_string().as_str())),
+        }
+    }
+}
+
+impl<T: PartialEq> PartialEq for FFIResult<T> {
     fn eq(&self, other: &Self) -> bool {
         match (self, other) {
-            (FFI_Result::Ok(a), FFI_Result::Ok(b)) => a == b,
-            (FFI_Result::Err(a), FFI_Result::Err(b)) => a == b,
+            (FFIResult::Ok(a), FFIResult::Ok(b)) => a == b,
+            (FFIResult::Err(a), FFIResult::Err(b)) => a == b,
             _ => false,
         }
     }

datafusion/ffi/src/physical_expr/mod.rs

@@ -46,7 +46,7 @@ use crate::expr::interval::FFI_Interval;
 use crate::record_batch_stream::{
     record_batch_to_wrapped_array, wrapped_array_to_record_batch,
 };
-use crate::util::{FFI_Option, FFI_Result, FFIResult};
+use crate::util::{FFI_Option, FFIResult};
 use crate::{df_result, sresult, sresult_return};
 
 #[repr(C)]
@@ -290,7 +290,7 @@ unsafe extern "C" fn propagate_constraints_fn_wrapper(
             .transpose()
     );
 
-    FFI_Result::Ok(result.into())
+    FFIResult::Ok(result.into())
 }
 
 unsafe extern "C" fn evaluate_statistics_fn_wrapper(
@@ -336,7 +336,7 @@ unsafe extern "C" fn propagate_statistics_fn_wrapper(
             .transpose()
     );
 
-    FFI_Result::Ok(result.into())
+    FFIResult::Ok(result.into())
 }
 
 unsafe extern "C" fn get_properties_fn_wrapper(
@@ -359,7 +359,7 @@ unsafe extern "C" fn get_properties_fn_wrapper(
 unsafe extern "C" fn fmt_sql_fn_wrapper(expr: &FFI_PhysicalExpr) -> FFIResult<SString> {
     let expr = expr.inner();
     let result = fmt_sql(expr.as_ref()).to_string();
-    FFI_Result::Ok(result.into())
+    FFIResult::Ok(result.into())
 }
 
 unsafe extern "C" fn snapshot_fn_wrapper(
@@ -683,8 +683,8 @@ impl PhysicalExpr for ForeignPhysicalExpr {
     fn fmt_sql(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         unsafe {
             match (self.expr.fmt_sql)(&self.expr) {
-                FFI_Result::Ok(sql) => write!(f, "{sql}"),
-                FFI_Result::Err(_) => Err(std::fmt::Error),
+                FFIResult::Ok(sql) => write!(f, "{sql}"),
+                FFIResult::Err(_) => Err(std::fmt::Error),
             }
         }
     }

datafusion/ffi/src/physical_optimizer.rs

@@ -28,7 +28,7 @@ use tokio::runtime::Handle;
 
 use crate::config::FFI_ConfigOptions;
 use crate::execution_plan::FFI_ExecutionPlan;
-use crate::util::{FFI_Result, FFIResult};
+use crate::util::{FFIResult};
 use crate::{df_result, sresult_return};
 
 /// A stable struct for sharing [`PhysicalOptimizerRule`] across FFI boundaries.
@@ -95,7 +95,7 @@ unsafe extern "C" fn optimize_fn_wrapper(
     let config = sresult_return!(ConfigOptions::try_from(config));
     let optimized_plan = sresult_return!(rule.optimize(plan, &config));
 
-    FFI_Result::Ok(FFI_ExecutionPlan::new(optimized_plan, runtime))
+    FFIResult::Ok(FFI_ExecutionPlan::new(optimized_plan, runtime))
 }
 
 unsafe extern "C" fn name_fn_wrapper(rule: &FFI_PhysicalOptimizerRule) -> SString {

datafusion/ffi/src/proto/logical_extension_codec.rs

@@ -44,7 +44,7 @@ use crate::table_provider::FFI_TableProvider;
 use crate::udaf::FFI_AggregateUDF;
 use crate::udf::FFI_ScalarUDF;
 use crate::udwf::FFI_WindowUDF;
-use crate::util::{FFI_Result, FFIResult};
+use crate::util::{FFIResult};
 use crate::{df_result, sresult_return};
 
 /// A stable struct for sharing [`LogicalExtensionCodec`] across FFI boundaries.
@@ -163,7 +163,7 @@ unsafe extern "C" fn try_decode_table_provider_fn_wrapper(
         ctx.as_ref()
     ));
 
-    FFI_Result::Ok(FFI_TableProvider::new_with_ffi_codec(
+    FFIResult::Ok(FFI_TableProvider::new_with_ffi_codec(
         table_provider,
         true,
         runtime,
@@ -187,7 +187,7 @@ unsafe extern "C" fn try_encode_table_provider_fn_wrapper(
         &mut bytes
     ));
 
-    FFI_Result::Ok(bytes.into_iter().collect())
+    FFIResult::Ok(bytes.into_iter().collect())
 }
 
 unsafe extern "C" fn try_decode_udf_fn_wrapper(
@@ -200,7 +200,7 @@ unsafe extern "C" fn try_decode_udf_fn_wrapper(
     let udf = sresult_return!(codec.try_decode_udf(name.as_str(), buf.as_ref()));
     let udf = FFI_ScalarUDF::from(udf);
 
-    FFI_Result::Ok(udf)
+    FFIResult::Ok(udf)
 }
 
 unsafe extern "C" fn try_encode_udf_fn_wrapper(
@@ -214,7 +214,7 @@ unsafe extern "C" fn try_encode_udf_fn_wrapper(
     let mut bytes = Vec::new();
     sresult_return!(codec.try_encode_udf(&node, &mut bytes));
 
-    FFI_Result::Ok(bytes.into_iter().collect())
+    FFIResult::Ok(bytes.into_iter().collect())
 }
 
 unsafe extern "C" fn try_decode_udaf_fn_wrapper(
@@ -226,7 +226,7 @@ unsafe extern "C" fn try_decode_udaf_fn_wrapper(
     let udaf = sresult_return!(codec_inner.try_decode_udaf(name.into(), buf.as_ref()));
     let udaf = FFI_AggregateUDF::from(udaf);
 
-    FFI_Result::Ok(udaf)
+    FFIResult::Ok(udaf)
 }
 
 unsafe extern "C" fn try_encode_udaf_fn_wrapper(
@@ -240,7 +240,7 @@ unsafe extern "C" fn try_encode_udaf_fn_wrapper(
     let mut bytes = Vec::new();
     sresult_return!(codec.try_encode_udaf(&udaf, &mut bytes));
 
-    FFI_Result::Ok(bytes.into_iter().collect())
+    FFIResult::Ok(bytes.into_iter().collect())
 }
 
 unsafe extern "C" fn try_decode_udwf_fn_wrapper(
@@ -252,7 +252,7 @@ unsafe extern "C" fn try_decode_udwf_fn_wrapper(
     let udwf = sresult_return!(codec.try_decode_udwf(name.into(), buf.as_ref()));
     let udwf = FFI_WindowUDF::from(udwf);
 
-    FFI_Result::Ok(udwf)
+    FFIResult::Ok(udwf)
 }
 
 unsafe extern "C" fn try_encode_udwf_fn_wrapper(
@@ -266,7 +266,7 @@ unsafe extern "C" fn try_encode_udwf_fn_wrapper(
     let mut bytes = Vec::new();
     sresult_return!(codec.try_encode_udwf(&udwf, &mut bytes));
 
-    FFI_Result::Ok(bytes.into_iter().collect())
+    FFIResult::Ok(bytes.into_iter().collect())
 }
 
 unsafe extern "C" fn release_fn_wrapper(provider: &mut FFI_LogicalExtensionCodec) {