diff --git a/Cargo.lock b/Cargo.lock index 8101e2fe22d92..57ab9945e0e5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,54 +2,6 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "abi_stable" -version = "0.11.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69d6512d3eb05ffe5004c59c206de7f99c34951504056ce23fc953842f12c445" -dependencies = [ - "abi_stable_derive", - "abi_stable_shared", - "const_panic", - "core_extensions", - "crossbeam-channel", - "generational-arena", - "libloading", - "lock_api", - "parking_lot", - "paste", - "repr_offset", - "rustc_version", - "serde", - "serde_derive", - "serde_json", -] - -[[package]] -name = "abi_stable_derive" -version = "0.11.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7178468b407a4ee10e881bc7a328a65e739f0863615cca4429d43916b05e898" -dependencies = [ - "abi_stable_shared", - "as_derive_utils", - "core_extensions", - "proc-macro2", - "quote", - "rustc_version", - "syn 1.0.109", - "typed-arena", -] - -[[package]] -name = "abi_stable_shared" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2b5df7688c123e63f4d4d649cba63f2967ba7f7861b1664fca3f77d3dad2b63" -dependencies = [ - "core_extensions", -] - [[package]] name = "adler2" version = "2.0.1" @@ -509,18 +461,6 @@ dependencies = [ "regex-syntax", ] -[[package]] -name = "as_derive_utils" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff3c96645900a44cf11941c111bd08a6573b0e2f9f69bc9264b179d8fae753c4" -dependencies = [ - "core_extensions", - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "astral-tokio-tar" version = "0.5.6" @@ -549,15 +489,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "async-ffi" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4de21c0feef7e5a556e51af767c953f0501f7f300ba785cc99c47bdc8081a50" -dependencies = [ - "abi_stable", -] - [[package]] name = "async-recursion" version = "1.1.1" @@ -1493,15 +1424,6 @@ dependencies = [ "tiny-keccak", ] -[[package]] -name = "const_panic" -version = "0.2.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e262cdaac42494e3ae34c43969f9cdeb7da178bdb4b66fa6a1ea2edb4c8ae652" -dependencies = [ - "typewit", -] - [[package]] name = "constant_time_eq" version = "0.4.2" @@ -1524,21 +1446,6 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" -[[package]] -name = "core_extensions" -version = "1.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42bb5e5d0269fd4f739ea6cedaf29c16d81c27a7ce7582008e90eb50dcd57003" -dependencies = [ - "core_extensions_proc_macros", -] - -[[package]] -name = "core_extensions_proc_macros" -version = "1.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "533d38ecd2709b7608fb8e18e4504deb99e9a72879e6aa66373a76d8dc4259ea" - [[package]] name = "cpufeatures" version = "0.2.17" @@ -1594,15 +1501,6 @@ dependencies = [ "itertools 0.13.0", ] -[[package]] -name = "crossbeam-channel" -version = "0.5.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82b8f8f868b36967f9606790d1903570de9ceaf870a7bf9fbbd3016d636a2cb2" -dependencies = [ - "crossbeam-utils", -] - [[package]] name = "crossbeam-deque" version = "0.8.6" @@ -2206,10 +2104,8 @@ dependencies = [ name = "datafusion-ffi" version = "52.3.0" dependencies = [ - "abi_stable", "arrow", "arrow-schema", - "async-ffi", "async-trait", "datafusion", "datafusion-catalog", @@ -2230,9 +2126,11 @@ dependencies = [ "datafusion-session", "doc-comment", "futures", + "libloading 0.8.9", "log", "prost", "semver", + "stabby", "tokio", ] @@ -2969,7 +2867,6 @@ dependencies = [ name = "ffi_example_table_provider" version = "0.1.0" dependencies = [ - "abi_stable", "arrow", "datafusion", "datafusion-ffi", @@ -2980,7 +2877,6 @@ dependencies = [ name = "ffi_module_interface" version = "0.1.0" dependencies = [ - "abi_stable", "datafusion-ffi", ] @@ -2988,10 +2884,10 @@ dependencies = [ name = "ffi_module_loader" version = "0.1.0" dependencies = [ - "abi_stable", "datafusion", "datafusion-ffi", "ffi_module_interface", + "libloading 0.8.9", "tokio", ] @@ -3191,15 +3087,6 @@ dependencies = [ "prost-build", ] -[[package]] -name = "generational-arena" -version = "0.2.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "877e94aff08e743b651baaea359664321055749b398adff8740a7399af7796e7" -dependencies = [ - "cfg-if", -] - [[package]] name = "generic-array" version = "0.14.7" @@ -3931,6 +3818,16 @@ dependencies = [ "winapi", ] +[[package]] +name = "libloading" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" +dependencies = [ + "cfg-if", + "windows-link", +] + [[package]] name = "liblzma" version = "0.4.6" @@ -5162,15 +5059,6 @@ version = "1.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" -[[package]] -name = "repr_offset" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb1070755bd29dffc19d0971cab794e607839ba2ef4b69a9e6fbc8733c1b72ea" -dependencies = [ - "tstr", -] - [[package]] name = "reqwest" version = "0.12.28" @@ -5656,6 +5544,12 @@ dependencies = [ "digest", ] +[[package]] +name = "sha2-const-stable" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f179d4e11094a893b82fff208f74d448a7512f99f5a0acbd5c679b705f83ed9" + [[package]] name = "sharded-slab" version = "0.1.7" @@ -5798,6 +5692,42 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "stabby" +version = "36.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "89b7e94eaf470c2e76b5f15fb2fb49714471a36cc512df5ee231e62e82ec79f8" +dependencies = [ + "libloading 0.7.4", + "rustversion", + "stabby-abi", +] + +[[package]] +name = "stabby-abi" +version = "36.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0dc7a63b8276b54e51bfffe3d85da56e7906b2dcfcb29018a8ab666c06734c1a" +dependencies = [ + "rustc_version", + "rustversion", + "sha2-const-stable", + "stabby-macros", +] + +[[package]] +name = "stabby-macros" +version = "36.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eecb7ec5611ec93ec79d120fbe55f31bea234dc1bed1001d4a071bb688651615" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "rand 0.8.5", + "syn 1.0.109", +] + [[package]] name = "stable_deref_trait" version = "1.2.1" @@ -6442,45 +6372,18 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" -[[package]] -name = "tstr" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f8e0294f14baae476d0dd0a2d780b2e24d66e349a9de876f5126777a37bdba7" -dependencies = [ - "tstr_proc_macros", -] - -[[package]] -name = "tstr_proc_macros" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e78122066b0cb818b8afd08f7ed22f7fdbc3e90815035726f0840d0d26c0747a" - [[package]] name = "twox-hash" version = "2.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ea3136b675547379c4bd395ca6b938e5ad3c3d20fad76e7fe85f9e0d011419c" -[[package]] -name = "typed-arena" -version = "2.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6af6ae20167a9ece4bcb41af5b80f8a1f1df981f6391189ce00fd257af04126a" - [[package]] name = "typenum" version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "562d481066bde0658276a35467c4af00bdc6ee726305698a55b86e61d7ad82bb" -[[package]] -name = "typewit" -version = "1.14.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8c1ae7cc0fdb8b842d65d127cb981574b0d2b249b74d1c7a2986863dc134f71" - [[package]] name = "typify" version = "0.5.0" diff --git a/datafusion-examples/examples/ffi/ffi_example_table_provider/Cargo.toml b/datafusion-examples/examples/ffi/ffi_example_table_provider/Cargo.toml index e2d0e3fa6744d..3cfa6dcf90f18 100644 --- a/datafusion-examples/examples/ffi/ffi_example_table_provider/Cargo.toml +++ b/datafusion-examples/examples/ffi/ffi_example_table_provider/Cargo.toml @@ -22,7 +22,6 @@ edition = { workspace = true } publish = false [dependencies] -abi_stable = "0.11.3" arrow = { workspace = true } datafusion = { workspace = true } datafusion-ffi = { workspace = true } diff --git a/datafusion-examples/examples/ffi/ffi_example_table_provider/src/lib.rs b/datafusion-examples/examples/ffi/ffi_example_table_provider/src/lib.rs index eb217ef9e4832..2d85d8c273b72 100644 --- a/datafusion-examples/examples/ffi/ffi_example_table_provider/src/lib.rs +++ b/datafusion-examples/examples/ffi/ffi_example_table_provider/src/lib.rs @@ -17,13 +17,12 @@ use std::sync::Arc; -use abi_stable::{export_root_module, prefix_type::PrefixTypeTrait}; use arrow::array::RecordBatch; use arrow::datatypes::{DataType, Field, Schema}; use datafusion::{common::record_batch, datasource::MemTable}; use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec; use datafusion_ffi::table_provider::FFI_TableProvider; -use ffi_module_interface::{TableProviderModule, TableProviderModuleRef}; +use ffi_module_interface::TableProviderModule; fn create_record_batch(start_value: i32, num_values: usize) -> RecordBatch { let end_value = start_value + num_values as i32; @@ -56,11 +55,10 @@ extern "C" fn construct_simple_table_provider( FFI_TableProvider::new_with_ffi_codec(Arc::new(table_provider), true, None, codec) } -#[export_root_module] /// This defines the entry point for using the module. -pub fn get_simple_memory_table() -> TableProviderModuleRef { +#[unsafe(no_mangle)] +pub extern "C" fn ffi_example_get_module() -> TableProviderModule { TableProviderModule { create_table: construct_simple_table_provider, } - .leak_into_prefix() } diff --git a/datafusion-examples/examples/ffi/ffi_module_interface/Cargo.toml b/datafusion-examples/examples/ffi/ffi_module_interface/Cargo.toml index fe4902711241e..0244cb2a5ed15 100644 --- a/datafusion-examples/examples/ffi/ffi_module_interface/Cargo.toml +++ b/datafusion-examples/examples/ffi/ffi_module_interface/Cargo.toml @@ -25,5 +25,4 @@ publish = false workspace = true [dependencies] -abi_stable = "0.11.3" datafusion-ffi = { workspace = true } diff --git a/datafusion-examples/examples/ffi/ffi_module_interface/src/lib.rs b/datafusion-examples/examples/ffi/ffi_module_interface/src/lib.rs index 3b2b9e1871dae..54a59c9e5d073 100644 --- a/datafusion-examples/examples/ffi/ffi_module_interface/src/lib.rs +++ b/datafusion-examples/examples/ffi/ffi_module_interface/src/lib.rs @@ -15,36 +15,17 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::{ - StableAbi, declare_root_module_statics, - library::{LibraryError, RootModule}, - package_version_strings, - sabi_types::VersionStrings, -}; use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec; use datafusion_ffi::table_provider::FFI_TableProvider; -#[repr(C)] -#[derive(StableAbi)] -#[sabi(kind(Prefix(prefix_ref = TableProviderModuleRef)))] /// This struct defines the module interfaces. It is to be shared by /// both the module loading program and library that implements the /// module. It is possible to move this definition into the loading /// program and reference it in the modules, but this example shows /// how a user may wish to separate these concerns. +#[repr(C)] pub struct TableProviderModule { /// Constructs the table provider pub create_table: extern "C" fn(codec: FFI_LogicalExtensionCodec) -> FFI_TableProvider, } - -impl RootModule for TableProviderModuleRef { - declare_root_module_statics! {TableProviderModuleRef} - const BASE_NAME: &'static str = "ffi_example_table_provider"; - const NAME: &'static str = "ffi_example_table_provider"; - const VERSION_STRINGS: VersionStrings = package_version_strings!(); - - fn initialization(self) -> Result { - Ok(self) - } -} diff --git a/datafusion-examples/examples/ffi/ffi_module_loader/Cargo.toml b/datafusion-examples/examples/ffi/ffi_module_loader/Cargo.toml index 8d7434dca211b..48821a9310769 100644 --- a/datafusion-examples/examples/ffi/ffi_module_loader/Cargo.toml +++ b/datafusion-examples/examples/ffi/ffi_module_loader/Cargo.toml @@ -25,8 +25,8 @@ publish = false workspace = true [dependencies] -abi_stable = "0.11.3" datafusion = { workspace = true } datafusion-ffi = { workspace = true } ffi_module_interface = { path = "../ffi_module_interface" } +libloading = "0.8" tokio = { workspace = true, features = ["rt-multi-thread", "parking_lot"] } diff --git a/datafusion-examples/examples/ffi/ffi_module_loader/src/main.rs b/datafusion-examples/examples/ffi/ffi_module_loader/src/main.rs index 8ce5b156df3b1..ef1cc4be2d951 100644 --- a/datafusion-examples/examples/ffi/ffi_module_loader/src/main.rs +++ b/datafusion-examples/examples/ffi/ffi_module_loader/src/main.rs @@ -18,28 +18,47 @@ use std::sync::Arc; use datafusion::{ + datasource::TableProvider, error::{DataFusionError, Result}, + execution::TaskContextProvider, prelude::SessionContext, }; - -use abi_stable::library::{RootModule, development_utils::compute_library_path}; -use datafusion::datasource::TableProvider; -use datafusion::execution::TaskContextProvider; use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec; -use ffi_module_interface::TableProviderModuleRef; +use ffi_module_interface::TableProviderModule; #[tokio::main] async fn main() -> Result<()> { // Find the location of the library. This is specific to the build environment, // so you will need to change the approach here based on your use case. - let target: &std::path::Path = "../../../../target/".as_ref(); - let library_path = compute_library_path::(target) - .map_err(|e| DataFusionError::External(Box::new(e)))?; + let lib_prefix = if cfg!(target_os = "windows") { + "" + } else { + "lib" + }; + let lib_ext = if cfg!(target_os = "macos") { + "dylib" + } else if cfg!(target_os = "windows") { + "dll" + } else { + "so" + }; + + let library_path = format!( + "../../../../target/debug/{lib_prefix}ffi_example_table_provider.{lib_ext}" + ); - // Load the module - let table_provider_module = - TableProviderModuleRef::load_from_directory(&library_path) - .map_err(|e| DataFusionError::External(Box::new(e)))?; + // Load the library using libloading + let lib = unsafe { + libloading::Library::new(&library_path) + .map_err(|e| DataFusionError::External(Box::new(e)))? + }; + + let get_module: libloading::Symbol TableProviderModule> = unsafe { + lib.get(b"ffi_example_get_module") + .map_err(|e| DataFusionError::External(Box::new(e)))? + }; + + let table_provider_module = get_module(); let ctx = Arc::new(SessionContext::new()); let codec = FFI_LogicalExtensionCodec::new_default( @@ -48,12 +67,7 @@ async fn main() -> Result<()> { // By calling the code below, the table provided will be created within // the module's code. - let ffi_table_provider = - table_provider_module - .create_table() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_table".to_string(), - ))?(codec); + let ffi_table_provider = (table_provider_module.create_table)(codec); // In order to access the table provider within this executable, we need to // turn it into a `TableProvider`. @@ -64,5 +78,8 @@ async fn main() -> Result<()> { let df = ctx.table("external_table").await?; df.show().await?; + // Keep the library loaded + std::mem::forget(lib); + Ok(()) } diff --git a/datafusion/ffi/Cargo.toml b/datafusion/ffi/Cargo.toml index 28e1b2ee5681f..242e939b6021e 100644 --- a/datafusion/ffi/Cargo.toml +++ b/datafusion/ffi/Cargo.toml @@ -44,11 +44,10 @@ crate-type = ["cdylib", "rlib"] # It increases build times and library binary size for users. [dependencies] -abi_stable = "0.11.3" arrow = { workspace = true, features = ["ffi"] } arrow-schema = { workspace = true } -async-ffi = { version = "0.5.0", features = ["abi_stable"] } async-trait = { workspace = true } +stabby = { version = "36", features = ["libloading"] } datafusion-catalog = { workspace = true } datafusion-common = { workspace = true } datafusion-datasource = { workspace = true } @@ -66,6 +65,7 @@ datafusion-proto = { workspace = true } datafusion-proto-common = { workspace = true } datafusion-session = { workspace = true } futures = { workspace = true } +libloading = "0.8" log = { workspace = true } prost = { workspace = true } semver = "1.0.27" diff --git a/datafusion/ffi/src/arrow_wrappers.rs b/datafusion/ffi/src/arrow_wrappers.rs index c83e412310e7f..ade9b4eae2169 100644 --- a/datafusion/ffi/src/arrow_wrappers.rs +++ b/datafusion/ffi/src/arrow_wrappers.rs @@ -17,7 +17,6 @@ use std::sync::Arc; -use abi_stable::StableAbi; use arrow::array::{ArrayRef, make_array}; use arrow::datatypes::{Schema, SchemaRef}; use arrow::error::ArrowError; @@ -26,10 +25,10 @@ use datafusion_common::{DataFusionError, ScalarValue}; use log::error; /// This is a wrapper struct around FFI_ArrowSchema simply to indicate -/// to the StableAbi macros that the underlying struct is FFI safe. +/// that the underlying struct is FFI safe. #[repr(C)] -#[derive(Debug, StableAbi)] -pub struct WrappedSchema(#[sabi(unsafe_opaque_field)] pub FFI_ArrowSchema); +#[derive(Debug)] +pub struct WrappedSchema(pub FFI_ArrowSchema); impl From for WrappedSchema { fn from(value: SchemaRef) -> Self { @@ -66,13 +65,12 @@ impl From for SchemaRef { } } -/// This is a wrapper struct for FFI_ArrowArray to indicate to StableAbi +/// This is a wrapper struct for FFI_ArrowArray to indicate /// that the struct is FFI Safe. For convenience, we also include the /// schema needed to create a record batch from the array. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct WrappedArray { - #[sabi(unsafe_opaque_field)] pub array: FFI_ArrowArray, pub schema: WrappedSchema, diff --git a/datafusion/ffi/src/catalog_provider.rs b/datafusion/ffi/src/catalog_provider.rs index 61e26f1663532..92475e325f837 100644 --- a/datafusion/ffi/src/catalog_provider.rs +++ b/datafusion/ffi/src/catalog_provider.rs @@ -19,13 +19,15 @@ use std::any::Any; use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RResult, RString, RVec}; +use crate::ffi_option::FfiOption; +use crate::ffi_option::FfiResult; use datafusion_catalog::{CatalogProvider, SchemaProvider}; use datafusion_common::error::Result; use datafusion_proto::logical_plan::{ DefaultLogicalExtensionCodec, LogicalExtensionCodec, }; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; use tokio::runtime::Handle; use crate::execution::FFI_TaskContextProvider; @@ -36,28 +38,28 @@ use crate::{df_result, rresult_return}; /// A stable struct for sharing [`CatalogProvider`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_CatalogProvider { - pub schema_names: unsafe extern "C" fn(provider: &Self) -> RVec, + pub schema_names: unsafe extern "C" fn(provider: &Self) -> SVec, pub schema: unsafe extern "C" fn( provider: &Self, - name: RString, - ) -> ROption, + name: SString, + ) -> FfiOption, pub register_schema: unsafe extern "C" fn( provider: &Self, - name: RString, + name: SString, schema: &FFI_SchemaProvider, ) - -> FFIResult>, + -> FFIResult>, - pub deregister_schema: unsafe extern "C" fn( - provider: &Self, - name: RString, - cascade: bool, - ) - -> FFIResult>, + pub deregister_schema: + unsafe extern "C" fn( + provider: &Self, + name: SString, + cascade: bool, + ) -> FFIResult>, pub logical_codec: FFI_LogicalExtensionCodec, @@ -107,17 +109,20 @@ impl FFI_CatalogProvider { unsafe extern "C" fn schema_names_fn_wrapper( provider: &FFI_CatalogProvider, -) -> RVec { +) -> SVec { unsafe { let names = provider.inner().schema_names(); - names.into_iter().map(|s| s.into()).collect() + names + .into_iter() + .map(|s| SString::from(s.as_str())) + .collect() } } unsafe extern "C" fn schema_fn_wrapper( provider: &FFI_CatalogProvider, - name: RString, -) -> ROption { + name: SString, +) -> FfiOption { unsafe { let maybe_schema = provider.inner().schema(name.as_str()); maybe_schema @@ -134,9 +139,9 @@ unsafe extern "C" fn schema_fn_wrapper( unsafe extern "C" fn register_schema_fn_wrapper( provider: &FFI_CatalogProvider, - name: RString, + name: SString, schema: &FFI_SchemaProvider, -) -> FFIResult> { +) -> FFIResult> { unsafe { let runtime = provider.runtime(); let inner_provider = provider.inner(); @@ -153,15 +158,15 @@ unsafe extern "C" fn register_schema_fn_wrapper( }) .into(); - RResult::ROk(returned_schema) + FfiResult::Ok(returned_schema) } } unsafe extern "C" fn deregister_schema_fn_wrapper( provider: &FFI_CatalogProvider, - name: RString, + name: SString, cascade: bool, -) -> FFIResult> { +) -> FFIResult> { unsafe { let runtime = provider.runtime(); let inner_provider = provider.inner(); @@ -169,7 +174,7 @@ unsafe extern "C" fn deregister_schema_fn_wrapper( let maybe_schema = rresult_return!(inner_provider.deregister_schema(name.as_str(), cascade)); - RResult::ROk( + FfiResult::Ok( maybe_schema .map(|schema| { FFI_SchemaProvider::new_with_ffi_codec( @@ -303,7 +308,7 @@ impl CatalogProvider for ForeignCatalogProvider { unsafe { (self.0.schema_names)(&self.0) .into_iter() - .map(|s| s.into()) + .map(|s| s.to_string()) .collect() } } @@ -311,7 +316,7 @@ impl CatalogProvider for ForeignCatalogProvider { fn schema(&self, name: &str) -> Option> { unsafe { let maybe_provider: Option = - (self.0.schema)(&self.0, name.into()).into(); + (self.0.schema)(&self.0, SString::from(name)).into(); maybe_provider.map(|provider| { Arc::new(ForeignSchemaProvider(provider)) as Arc @@ -334,8 +339,12 @@ impl CatalogProvider for ForeignCatalogProvider { ), }; let returned_schema: Option = - df_result!((self.0.register_schema)(&self.0, name.into(), schema))? - .into(); + df_result!((self.0.register_schema)( + &self.0, + SString::from(name), + schema + ))? + .into(); Ok(returned_schema .map(|s| Arc::new(ForeignSchemaProvider(s)) as Arc)) @@ -349,8 +358,12 @@ impl CatalogProvider for ForeignCatalogProvider { ) -> Result>> { unsafe { let returned_schema: Option = - df_result!((self.0.deregister_schema)(&self.0, name.into(), cascade))? - .into(); + df_result!((self.0.deregister_schema)( + &self.0, + SString::from(name), + cascade + ))? + .into(); Ok(returned_schema .map(|s| Arc::new(ForeignSchemaProvider(s)) as Arc)) diff --git a/datafusion/ffi/src/catalog_provider_list.rs b/datafusion/ffi/src/catalog_provider_list.rs index 40f8be3871bb9..9c0136187bd77 100644 --- a/datafusion/ffi/src/catalog_provider_list.rs +++ b/datafusion/ffi/src/catalog_provider_list.rs @@ -19,12 +19,13 @@ use std::any::Any; use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RString, RVec}; +use crate::ffi_option::FfiOption; use datafusion_catalog::{CatalogProvider, CatalogProviderList}; use datafusion_proto::logical_plan::{ DefaultLogicalExtensionCodec, LogicalExtensionCodec, }; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; use tokio::runtime::Handle; use crate::catalog_provider::{FFI_CatalogProvider, ForeignCatalogProvider}; @@ -33,21 +34,21 @@ use crate::proto::logical_extension_codec::FFI_LogicalExtensionCodec; /// A stable struct for sharing [`CatalogProviderList`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_CatalogProviderList { /// Register a catalog pub register_catalog: unsafe extern "C" fn( &Self, - name: RString, + name: SString, catalog: &FFI_CatalogProvider, - ) -> ROption, + ) -> FfiOption, /// List of existing catalogs - pub catalog_names: unsafe extern "C" fn(&Self) -> RVec, + pub catalog_names: unsafe extern "C" fn(&Self) -> SVec, /// Access a catalog pub catalog: - unsafe extern "C" fn(&Self, name: RString) -> ROption, + unsafe extern "C" fn(&Self, name: SString) -> FfiOption, pub logical_codec: FFI_LogicalExtensionCodec, @@ -97,25 +98,28 @@ impl FFI_CatalogProviderList { unsafe extern "C" fn catalog_names_fn_wrapper( provider: &FFI_CatalogProviderList, -) -> RVec { +) -> SVec { unsafe { let names = provider.inner().catalog_names(); - names.into_iter().map(|s| s.into()).collect() + names + .into_iter() + .map(|s| SString::from(s.as_str())) + .collect() } } unsafe extern "C" fn register_catalog_fn_wrapper( provider: &FFI_CatalogProviderList, - name: RString, + name: SString, catalog: &FFI_CatalogProvider, -) -> ROption { +) -> FfiOption { unsafe { let runtime = provider.runtime(); let inner_provider = provider.inner(); let catalog: Arc = catalog.into(); inner_provider - .register_catalog(name.into(), catalog) + .register_catalog(name.to_string(), catalog) .map(|catalog| { FFI_CatalogProvider::new_with_ffi_codec( catalog, @@ -129,8 +133,8 @@ unsafe extern "C" fn register_catalog_fn_wrapper( unsafe extern "C" fn catalog_fn_wrapper( provider: &FFI_CatalogProviderList, - name: RString, -) -> ROption { + name: SString, +) -> FfiOption { unsafe { let runtime = provider.runtime(); let inner_provider = provider.inner(); @@ -276,7 +280,7 @@ impl CatalogProviderList for ForeignCatalogProviderList { ), }; - (self.0.register_catalog)(&self.0, name.into(), catalog) + (self.0.register_catalog)(&self.0, SString::from(name.as_str()), catalog) .map(|s| Arc::new(ForeignCatalogProvider(s)) as Arc) .into() } @@ -286,14 +290,14 @@ impl CatalogProviderList for ForeignCatalogProviderList { unsafe { (self.0.catalog_names)(&self.0) .into_iter() - .map(Into::into) + .map(|s| s.to_string()) .collect() } } fn catalog(&self, name: &str) -> Option> { unsafe { - (self.0.catalog)(&self.0, name.into()) + (self.0.catalog)(&self.0, SString::from(name)) .map(|catalog| { Arc::new(ForeignCatalogProvider(catalog)) as Arc }) diff --git a/datafusion/ffi/src/config/extension_options.rs b/datafusion/ffi/src/config/extension_options.rs index 48fd4e710921a..cccdf7d4d7b68 100644 --- a/datafusion/ffi/src/config/extension_options.rs +++ b/datafusion/ffi/src/config/extension_options.rs @@ -19,8 +19,11 @@ use std::any::Any; use std::collections::HashMap; use std::ffi::c_void; -use abi_stable::StableAbi; -use abi_stable::std_types::{RResult, RStr, RString, RVec, Tuple2}; +use crate::ffi_option::FfiResult; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; +use stabby::str::Str as SStr; + use datafusion_common::config::{ConfigEntry, ConfigExtension, ExtensionOptions}; use datafusion_common::{Result, exec_err}; @@ -38,17 +41,17 @@ use crate::df_result; /// are stored with the full path prefix to avoid overwriting values when using /// multiple extensions. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_ExtensionOptions { /// Return a deep clone of this [`ExtensionOptions`] pub cloned: unsafe extern "C" fn(&Self) -> FFI_ExtensionOptions, /// Set the given `key`, `value` pair pub set: - unsafe extern "C" fn(&mut Self, key: RStr, value: RStr) -> RResult<(), RString>, + unsafe extern "C" fn(&mut Self, key: SStr, value: SStr) -> FfiResult<(), SString>, /// Returns the [`ConfigEntry`] stored in this [`ExtensionOptions`] - pub entries: unsafe extern "C" fn(&Self) -> RVec>, + pub entries: unsafe extern "C" fn(&Self) -> SVec<(SString, SString)>, /// Release the memory of the private data when it is no longer being used. pub release: unsafe extern "C" fn(&mut Self), @@ -91,20 +94,22 @@ unsafe extern "C" fn cloned_fn_wrapper( unsafe extern "C" fn set_fn_wrapper( options: &mut FFI_ExtensionOptions, - key: RStr, - value: RStr, -) -> RResult<(), RString> { - let _ = options.inner_mut().insert(key.into(), value.into()); - RResult::ROk(()) + key: SStr, + value: SStr, +) -> FfiResult<(), SString> { + let _ = options + .inner_mut() + .insert(key.as_str().into(), value.as_str().into()); + FfiResult::Ok(()) } unsafe extern "C" fn entries_fn_wrapper( options: &FFI_ExtensionOptions, -) -> RVec> { +) -> SVec<(SString, SString)> { options .inner() .iter() - .map(|(key, value)| (key.to_owned().into(), value.to_owned().into()).into()) + .map(|(key, value)| (SString::from(key.as_str()), SString::from(value.as_str()))) .collect() } @@ -181,9 +186,9 @@ impl ExtensionOptions for FFI_ExtensionOptions { unsafe { (self.entries)(self) .into_iter() - .map(|entry_tuple| ConfigEntry { - key: entry_tuple.0.into(), - value: Some(entry_tuple.1.into()), + .map(|(key, value)| ConfigEntry { + key: key.to_string(), + value: Some(value.to_string()), description: "ffi_config_options", }) .collect() @@ -223,9 +228,9 @@ impl FFI_ExtensionOptions { let mut result = C::default(); unsafe { - for entry in (self.entries)(self) { - let key = entry.0.as_str(); - let value = entry.1.as_str(); + for (key, value) in (self.entries)(self) { + let key = key.as_str(); + let value = value.as_str(); if let Some((prefix, inner_key)) = key.split_once('.') && prefix == C::PREFIX diff --git a/datafusion/ffi/src/config/mod.rs b/datafusion/ffi/src/config/mod.rs index 850a4dc337336..f389ba3af82dc 100644 --- a/datafusion/ffi/src/config/mod.rs +++ b/datafusion/ffi/src/config/mod.rs @@ -17,8 +17,9 @@ pub mod extension_options; -use abi_stable::StableAbi; -use abi_stable::std_types::{RHashMap, RString}; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; + use datafusion_common::config::{ ConfigExtension, ConfigOptions, ExtensionOptions, TableOptions, }; @@ -32,20 +33,22 @@ use crate::config::extension_options::FFI_ExtensionOptions; /// than local extensions. The trait [`ExtensionOptionsFFIProvider`] can /// be used to simplify accessing FFI extensions. #[repr(C)] -#[derive(Debug, Clone, StableAbi)] +#[derive(Debug, Clone)] pub struct FFI_ConfigOptions { - base_options: RHashMap, + base_options: SVec<(SString, SString)>, extensions: FFI_ExtensionOptions, } impl From<&ConfigOptions> for FFI_ConfigOptions { fn from(options: &ConfigOptions) -> Self { - let base_options: RHashMap = options + let base_options: SVec<(SString, SString)> = options .entries() .into_iter() .filter_map(|entry| entry.value.map(|value| (entry.key, value))) - .map(|(key, value)| (key.into(), value.into())) + .map(|(key, value)| { + (SString::from(key.as_str()), SString::from(value.as_str())) + }) .collect(); let mut extensions = FFI_ExtensionOptions::default(); @@ -72,8 +75,8 @@ impl TryFrom for ConfigOptions { let mut options = ConfigOptions::default(); options.extensions.insert(ffi_options.extensions); - for kv_tuple in ffi_options.base_options.iter() { - options.set(kv_tuple.0.as_str(), kv_tuple.1.as_str())?; + for (key, value) in ffi_options.base_options.iter() { + options.set(key.as_str(), value.as_str())?; } Ok(options) @@ -120,20 +123,22 @@ impl ExtensionOptionsFFIProvider for TableOptions { /// than local extensions. The trait [`ExtensionOptionsFFIProvider`] can /// be used to simplify accessing FFI extensions. #[repr(C)] -#[derive(Debug, Clone, StableAbi)] +#[derive(Debug, Clone)] pub struct FFI_TableOptions { - base_options: RHashMap, + base_options: SVec<(SString, SString)>, extensions: FFI_ExtensionOptions, } impl From<&TableOptions> for FFI_TableOptions { fn from(options: &TableOptions) -> Self { - let base_options: RHashMap = options + let base_options: SVec<(SString, SString)> = options .entries() .into_iter() .filter_map(|entry| entry.value.map(|value| (entry.key, value))) - .map(|(key, value)| (key.into(), value.into())) + .map(|(key, value)| { + (SString::from(key.as_str()), SString::from(value.as_str())) + }) .collect(); let mut extensions = FFI_ExtensionOptions::default(); @@ -160,8 +165,8 @@ impl TryFrom for TableOptions { let mut options = TableOptions::default(); options.extensions.insert(ffi_options.extensions); - for kv_tuple in ffi_options.base_options.iter() { - options.set(kv_tuple.0.as_str(), kv_tuple.1.as_str())?; + for (key, value) in ffi_options.base_options.iter() { + options.set(key.as_str(), value.as_str())?; } Ok(options) diff --git a/datafusion/ffi/src/execution/task_ctx.rs b/datafusion/ffi/src/execution/task_ctx.rs index e0598db0a0170..a743c1ee6ca3a 100644 --- a/datafusion/ffi/src/execution/task_ctx.rs +++ b/datafusion/ffi/src/execution/task_ctx.rs @@ -18,15 +18,15 @@ use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::pmr::ROption; -use abi_stable::std_types::{RHashMap, RString}; +use crate::ffi_option::FfiOption; use datafusion_execution::TaskContext; use datafusion_execution::config::SessionConfig; use datafusion_execution::runtime_env::RuntimeEnv; use datafusion_expr::{ AggregateUDF, AggregateUDFImpl, ScalarUDF, ScalarUDFImpl, WindowUDF, WindowUDFImpl, }; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; use crate::session::config::FFI_SessionConfig; use crate::udaf::FFI_AggregateUDF; @@ -35,26 +35,26 @@ use crate::udwf::FFI_WindowUDF; /// A stable struct for sharing [`TaskContext`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_TaskContext { /// Return the session ID. - pub session_id: unsafe extern "C" fn(&Self) -> RString, + pub session_id: unsafe extern "C" fn(&Self) -> SString, /// Return the task ID. - pub task_id: unsafe extern "C" fn(&Self) -> ROption, + pub task_id: unsafe extern "C" fn(&Self) -> FfiOption, /// Return the session configuration. pub session_config: unsafe extern "C" fn(&Self) -> FFI_SessionConfig, - /// Returns a hashmap of names to scalar functions. - pub scalar_functions: unsafe extern "C" fn(&Self) -> RHashMap, + /// Returns a vec of name-function pairs for scalar functions. + pub scalar_functions: unsafe extern "C" fn(&Self) -> SVec<(SString, FFI_ScalarUDF)>, - /// Returns a hashmap of names to aggregate functions. + /// Returns a vec of name-function pairs for aggregate functions. pub aggregate_functions: - unsafe extern "C" fn(&Self) -> RHashMap, + unsafe extern "C" fn(&Self) -> SVec<(SString, FFI_AggregateUDF)>, - /// Returns a hashmap of names to window functions. - pub window_functions: unsafe extern "C" fn(&Self) -> RHashMap, + /// Returns a vec of name-function pairs for window functions. + pub window_functions: unsafe extern "C" fn(&Self) -> SVec<(SString, FFI_WindowUDF)>, /// Release the memory of the private data when it is no longer being used. pub release: unsafe extern "C" fn(arg: &mut Self), @@ -82,17 +82,20 @@ impl FFI_TaskContext { } } -unsafe extern "C" fn session_id_fn_wrapper(ctx: &FFI_TaskContext) -> RString { +unsafe extern "C" fn session_id_fn_wrapper(ctx: &FFI_TaskContext) -> SString { unsafe { let ctx = ctx.inner(); - ctx.session_id().into() + SString::from(ctx.session_id()) } } -unsafe extern "C" fn task_id_fn_wrapper(ctx: &FFI_TaskContext) -> ROption { +unsafe extern "C" fn task_id_fn_wrapper(ctx: &FFI_TaskContext) -> FfiOption { unsafe { let ctx = ctx.inner(); - ctx.task_id().map(|s| s.as_str().into()).into() + match ctx.task_id() { + Some(s) => FfiOption::Some(SString::from(s.as_str())), + None => FfiOption::None, + } } } @@ -107,26 +110,26 @@ unsafe extern "C" fn session_config_fn_wrapper( unsafe extern "C" fn scalar_functions_fn_wrapper( ctx: &FFI_TaskContext, -) -> RHashMap { +) -> SVec<(SString, FFI_ScalarUDF)> { unsafe { let ctx = ctx.inner(); ctx.scalar_functions() .iter() - .map(|(name, udf)| (name.to_owned().into(), Arc::clone(udf).into())) + .map(|(name, udf)| (SString::from(name.as_str()), Arc::clone(udf).into())) .collect() } } unsafe extern "C" fn aggregate_functions_fn_wrapper( ctx: &FFI_TaskContext, -) -> RHashMap { +) -> SVec<(SString, FFI_AggregateUDF)> { unsafe { let ctx = ctx.inner(); ctx.aggregate_functions() .iter() .map(|(name, udaf)| { ( - name.to_owned().into(), + SString::from(name.as_str()), FFI_AggregateUDF::from(Arc::clone(udaf)), ) }) @@ -136,13 +139,16 @@ unsafe extern "C" fn aggregate_functions_fn_wrapper( unsafe extern "C" fn window_functions_fn_wrapper( ctx: &FFI_TaskContext, -) -> RHashMap { +) -> SVec<(SString, FFI_WindowUDF)> { unsafe { let ctx = ctx.inner(); ctx.window_functions() .iter() .map(|(name, udf)| { - (name.to_owned().into(), FFI_WindowUDF::from(Arc::clone(udf))) + ( + SString::from(name.as_str()), + FFI_WindowUDF::from(Arc::clone(udf)), + ) }) .collect() } @@ -186,41 +192,42 @@ impl From for Arc { return Arc::clone(ffi_ctx.inner()); } - let task_id = (ffi_ctx.task_id)(&ffi_ctx).map(|s| s.to_string()).into(); - let session_id = (ffi_ctx.session_id)(&ffi_ctx).into(); + let task_id: Option = (ffi_ctx.task_id)(&ffi_ctx).into(); + let task_id = task_id.map(|s| s.to_string()); + let session_id = (ffi_ctx.session_id)(&ffi_ctx).to_string(); let session_config = (ffi_ctx.session_config)(&ffi_ctx); let session_config = SessionConfig::try_from(&session_config).unwrap_or_default(); let scalar_functions = (ffi_ctx.scalar_functions)(&ffi_ctx) .into_iter() - .map(|kv_pair| { - let udf = >::from(&kv_pair.1); + .map(|(name, ffi_udf)| { + let udf = >::from(&ffi_udf); ( - kv_pair.0.into_string(), + name.to_string(), Arc::new(ScalarUDF::new_from_shared_impl(udf)), ) }) .collect(); let aggregate_functions = (ffi_ctx.aggregate_functions)(&ffi_ctx) .into_iter() - .map(|kv_pair| { - let udaf = >::from(&kv_pair.1); + .map(|(name, ffi_udaf)| { + let udaf = >::from(&ffi_udaf); ( - kv_pair.0.into_string(), + name.to_string(), Arc::new(AggregateUDF::new_from_shared_impl(udaf)), ) }) .collect(); let window_functions = (ffi_ctx.window_functions)(&ffi_ctx) .into_iter() - .map(|kv_pair| { - let udwf = >::from(&kv_pair.1); + .map(|(name, ffi_udwf)| { + let udwf = >::from(&ffi_udwf); ( - kv_pair.0.into_string(), + name.to_string(), Arc::new(WindowUDF::new_from_shared_impl(udwf)), ) }) diff --git a/datafusion/ffi/src/execution/task_ctx_provider.rs b/datafusion/ffi/src/execution/task_ctx_provider.rs index 5d4eaac83975a..6ab010f4fb97e 100644 --- a/datafusion/ffi/src/execution/task_ctx_provider.rs +++ b/datafusion/ffi/src/execution/task_ctx_provider.rs @@ -18,7 +18,6 @@ use std::ffi::c_void; use std::sync::{Arc, Weak}; -use abi_stable::StableAbi; use datafusion_common::{DataFusionError, ffi_datafusion_err}; use datafusion_execution::{TaskContext, TaskContextProvider}; @@ -32,7 +31,7 @@ use crate::{df_result, rresult}; /// data passed across the FFI boundary. See the crate README for /// additional information. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_TaskContextProvider { /// Retrieve the current [`TaskContext`] provided the provider has not /// gone out of scope. This function will return an error if the weakly diff --git a/datafusion/ffi/src/execution_plan.rs b/datafusion/ffi/src/execution_plan.rs index 064e4a895317b..9b2b7e02d88e1 100644 --- a/datafusion/ffi/src/execution_plan.rs +++ b/datafusion/ffi/src/execution_plan.rs @@ -19,14 +19,14 @@ use std::ffi::c_void; use std::pin::Pin; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{RString, RVec}; use datafusion_common::tree_node::TreeNodeRecursion; use datafusion_common::{DataFusionError, Result}; use datafusion_execution::{SendableRecordBatchStream, TaskContext}; use datafusion_physical_plan::{ DisplayAs, DisplayFormatType, ExecutionPlan, PlanProperties, }; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; use tokio::runtime::Handle; use crate::execution::FFI_TaskContext; @@ -37,16 +37,16 @@ use crate::{df_result, rresult}; /// A stable struct for sharing a [`ExecutionPlan`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_ExecutionPlan { /// Return the plan properties pub properties: unsafe extern "C" fn(plan: &Self) -> FFI_PlanProperties, /// Return a vector of children plans - pub children: unsafe extern "C" fn(plan: &Self) -> RVec, + pub children: unsafe extern "C" fn(plan: &Self) -> SVec, /// Return the plan name. - pub name: unsafe extern "C" fn(plan: &Self) -> RString, + pub name: unsafe extern "C" fn(plan: &Self) -> SString, /// Execute the plan and return a record batch stream. Errors /// will be returned as a string. @@ -96,19 +96,16 @@ unsafe extern "C" fn properties_fn_wrapper( unsafe extern "C" fn children_fn_wrapper( plan: &FFI_ExecutionPlan, -) -> RVec { +) -> SVec { unsafe { let private_data = plan.private_data as *const ExecutionPlanPrivateData; let plan = &(*private_data).plan; let runtime = &(*private_data).runtime; - let children: Vec<_> = plan - .children() + plan.children() .into_iter() .map(|child| FFI_ExecutionPlan::new(Arc::clone(child), runtime.clone())) - .collect(); - - children.into() + .collect() } } @@ -130,8 +127,8 @@ unsafe extern "C" fn execute_fn_wrapper( } } -unsafe extern "C" fn name_fn_wrapper(plan: &FFI_ExecutionPlan) -> RString { - plan.inner().name().into() +unsafe extern "C" fn name_fn_wrapper(plan: &FFI_ExecutionPlan) -> SString { + SString::from(plan.inner().name()) } unsafe extern "C" fn release_fn_wrapper(plan: &mut FFI_ExecutionPlan) { @@ -232,7 +229,7 @@ impl TryFrom<&FFI_ExecutionPlan> for Arc { } unsafe { - let name = (plan.name)(plan).into(); + let name = (plan.name)(plan).to_string(); let properties: PlanProperties = (plan.properties)(plan).try_into()?; diff --git a/datafusion/ffi/src/expr/columnar_value.rs b/datafusion/ffi/src/expr/columnar_value.rs index 7ad7645ecb6cf..c0090e3f826a3 100644 --- a/datafusion/ffi/src/expr/columnar_value.rs +++ b/datafusion/ffi/src/expr/columnar_value.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; use datafusion_common::{DataFusionError, ScalarValue}; use datafusion_expr::ColumnarValue; @@ -24,7 +23,7 @@ use crate::arrow_wrappers::WrappedArray; /// A stable struct for sharing [`ColumnarValue`] across FFI boundaries. /// Scalar values are passed as an Arrow array of length 1. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub enum FFI_ColumnarValue { Array(WrappedArray), Scalar(WrappedArray), diff --git a/datafusion/ffi/src/expr/distribution.rs b/datafusion/ffi/src/expr/distribution.rs index b9ebfc2362c7a..ca760f16ad17c 100644 --- a/datafusion/ffi/src/expr/distribution.rs +++ b/datafusion/ffi/src/expr/distribution.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; use datafusion_common::DataFusionError; use datafusion_expr::statistics::{ BernoulliDistribution, Distribution, ExponentialDistribution, GaussianDistribution, @@ -28,7 +27,7 @@ use crate::expr::interval::FFI_Interval; /// A stable struct for sharing [`Distribution`] across FFI boundaries. /// See ['Distribution'] for the meaning of each variant. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] #[expect(clippy::large_enum_variant)] pub enum FFI_Distribution { Uniform(FFI_UniformDistribution), @@ -67,13 +66,13 @@ impl TryFrom for Distribution { } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_UniformDistribution { interval: FFI_Interval, } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_ExponentialDistribution { rate: WrappedArray, offset: WrappedArray, @@ -81,20 +80,20 @@ pub struct FFI_ExponentialDistribution { } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_GaussianDistribution { mean: WrappedArray, variance: WrappedArray, } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_BernoulliDistribution { p: WrappedArray, } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_GenericDistribution { mean: WrappedArray, median: WrappedArray, diff --git a/datafusion/ffi/src/expr/expr_properties.rs b/datafusion/ffi/src/expr/expr_properties.rs index 199a399a6471f..5b37cc6a28535 100644 --- a/datafusion/ffi/src/expr/expr_properties.rs +++ b/datafusion/ffi/src/expr/expr_properties.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; use arrow_schema::SortOptions; use datafusion_common::DataFusionError; use datafusion_expr::sort_properties::{ExprProperties, SortProperties}; @@ -25,7 +24,7 @@ use crate::expr::interval::FFI_Interval; /// A stable struct for sharing [`ExprProperties`] across FFI boundaries. /// See [`ExprProperties`] for the meaning of each field. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_ExprProperties { sort_properties: FFI_SortProperties, range: FFI_Interval, @@ -60,7 +59,7 @@ impl TryFrom for ExprProperties { } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub enum FFI_SortProperties { Ordered(FFI_SortOptions), Unordered, @@ -88,7 +87,7 @@ impl From<&FFI_SortProperties> for SortProperties { } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_SortOptions { pub descending: bool, pub nulls_first: bool, diff --git a/datafusion/ffi/src/expr/interval.rs b/datafusion/ffi/src/expr/interval.rs index 450f3747a57f0..6334f7bb24d90 100644 --- a/datafusion/ffi/src/expr/interval.rs +++ b/datafusion/ffi/src/expr/interval.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; use datafusion_common::DataFusionError; use datafusion_expr::interval_arithmetic::Interval; @@ -25,7 +24,7 @@ use crate::arrow_wrappers::WrappedArray; /// See [`Interval`] for the meaning of each field. Scalar values /// are passed as Arrow arrays of length 1. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_Interval { lower: WrappedArray, upper: WrappedArray, diff --git a/datafusion/ffi/src/ffi_future.rs b/datafusion/ffi/src/ffi_future.rs new file mode 100644 index 0000000000000..83814a14a2143 --- /dev/null +++ b/datafusion/ffi/src/ffi_future.rs @@ -0,0 +1,145 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! FFI-safe async primitives, replacing the `async-ffi` crate. + +use std::ffi::c_void; +use std::future::Future; +use std::pin::Pin; +use std::task::{Context, Poll}; + +/// FFI-safe version of [`Poll`]. +#[repr(C)] +pub enum FfiPoll { + Ready(T), + Pending, + Panicked, +} + +/// FFI-safe wrapper around [`Context`] for passing wakers across FFI boundaries. +#[repr(C)] +pub struct FfiContext { + data: *mut c_void, +} + +impl FfiContext { + /// Borrow back the original [`Context`]. + /// + /// # Safety + /// + /// The caller must ensure the pointer is valid and derived from a real `Context`. + pub unsafe fn with_context(&mut self, f: F) -> R + where + F: FnOnce(&mut Context<'_>) -> R, + { + let cx = unsafe { &mut *(self.data as *mut Context<'_>) }; + f(cx) + } +} + +/// Extension trait on [`Context`] to produce an [`FfiContext`]. +pub trait ContextExt { + fn with_ffi_context(&mut self, f: F) -> R + where + F: FnOnce(&mut FfiContext) -> R; +} + +impl ContextExt for Context<'_> { + fn with_ffi_context(&mut self, f: F) -> R + where + F: FnOnce(&mut FfiContext) -> R, + { + let mut ffi_cx = FfiContext { + data: self as *mut Context<'_> as *mut c_void, + }; + f(&mut ffi_cx) + } +} + +/// An FFI-safe future. Wraps a boxed, pinned, `Send + 'static` future behind +/// `extern "C"` function pointers so it can cross shared-library boundaries. +#[repr(C)] +pub struct FfiFuture { + poll_fn: unsafe extern "C" fn(data: *mut c_void, context: *mut c_void) -> FfiPoll, + drop_fn: unsafe extern "C" fn(data: *mut c_void), + data: *mut c_void, +} + +unsafe impl Send for FfiFuture {} + +impl FfiFuture { + fn new + Send + 'static>(future: F) -> Self { + let boxed: Box + Send>>> = + Box::new(Box::pin(future)); + let data = Box::into_raw(boxed) as *mut c_void; + FfiFuture { + poll_fn: poll_wrapper::, + drop_fn: drop_wrapper::, + data, + } + } +} + +unsafe extern "C" fn poll_wrapper( + data: *mut c_void, + context: *mut c_void, +) -> FfiPoll { + let future = unsafe { &mut *(data as *mut Pin + Send>>) }; + let cx = unsafe { &mut *(context as *mut Context<'_>) }; + match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + future.as_mut().poll(cx) + })) { + Ok(Poll::Ready(v)) => FfiPoll::Ready(v), + Ok(Poll::Pending) => FfiPoll::Pending, + Err(_) => FfiPoll::Panicked, + } +} + +unsafe extern "C" fn drop_wrapper(data: *mut c_void) { + drop(unsafe { Box::from_raw(data as *mut Pin + Send>>) }); +} + +impl Future for FfiFuture { + type Output = T; + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let this = self.get_mut(); + let result = + unsafe { (this.poll_fn)(this.data, cx as *mut Context<'_> as *mut c_void) }; + match result { + FfiPoll::Ready(v) => Poll::Ready(v), + FfiPoll::Pending => Poll::Pending, + FfiPoll::Panicked => { + panic!("Panic occurred during poll on FfiFuture") + } + } + } +} + +impl Drop for FfiFuture { + fn drop(&mut self) { + unsafe { (self.drop_fn)(self.data) } + } +} + +/// Extension trait to convert any `Future + Send + 'static` into an [`FfiFuture`]. +pub trait FutureExt: Future + Send + 'static + Sized { + fn into_ffi(self) -> FfiFuture { + FfiFuture::new(self) + } +} + +impl FutureExt for F {} diff --git a/datafusion/ffi/src/ffi_option.rs b/datafusion/ffi/src/ffi_option.rs new file mode 100644 index 0000000000000..b998336ef4e2e --- /dev/null +++ b/datafusion/ffi/src/ffi_option.rs @@ -0,0 +1,126 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! FFI-safe Option and Result types that do not require `IStable` bounds. +//! +//! stabby's `Option` and `Result` require `T: IStable` for niche +//! optimization. Many of our FFI structs contain self-referential function +//! pointers and cannot implement `IStable`. These simple `#[repr(C)]` types +//! provide the same FFI-safe semantics without that constraint. + +/// An FFI-safe option type. +#[repr(C, u8)] +#[derive(Debug, Clone)] +pub enum FfiOption { + Some(T), + None, +} + +impl From> for FfiOption { + fn from(opt: Option) -> Self { + match opt { + Some(v) => FfiOption::Some(v), + None => FfiOption::None, + } + } +} + +impl From> for Option { + fn from(opt: FfiOption) -> Self { + match opt { + FfiOption::Some(v) => Some(v), + FfiOption::None => None, + } + } +} + +impl FfiOption { + pub fn as_ref(&self) -> Option<&T> { + match self { + FfiOption::Some(v) => Some(v), + FfiOption::None => None, + } + } + + pub fn map U>(self, f: F) -> FfiOption { + match self { + FfiOption::Some(v) => FfiOption::Some(f(v)), + FfiOption::None => FfiOption::None, + } + } + + pub fn into_option(self) -> Option { + self.into() + } +} + +/// An FFI-safe result type. +#[repr(C, u8)] +#[derive(Debug, Clone)] +pub enum FfiResult { + Ok(T), + Err(E), +} + +impl From> for FfiResult { + fn from(res: Result) -> Self { + match res { + Ok(v) => FfiResult::Ok(v), + Err(e) => FfiResult::Err(e), + } + } +} + +impl From> for Result { + fn from(res: FfiResult) -> Self { + match res { + FfiResult::Ok(v) => Ok(v), + FfiResult::Err(e) => Err(e), + } + } +} + +impl FfiResult { + pub fn is_ok(&self) -> bool { + matches!(self, FfiResult::Ok(_)) + } + + pub fn is_err(&self) -> bool { + matches!(self, FfiResult::Err(_)) + } + + pub fn unwrap_err(self) -> E { + match self { + FfiResult::Err(e) => e, + FfiResult::Ok(_) => panic!("called unwrap_err on Ok"), + } + } + + pub fn into_result(self) -> Result { + self.into() + } +} + +impl PartialEq for FfiResult { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (FfiResult::Ok(a), FfiResult::Ok(b)) => a == b, + (FfiResult::Err(a), FfiResult::Err(b)) => a == b, + _ => false, + } + } +} diff --git a/datafusion/ffi/src/insert_op.rs b/datafusion/ffi/src/insert_op.rs index 6471039105e80..3eab4ad7985d7 100644 --- a/datafusion/ffi/src/insert_op.rs +++ b/datafusion/ffi/src/insert_op.rs @@ -15,12 +15,12 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; use datafusion_expr::logical_plan::dml::InsertOp; /// FFI safe version of [`InsertOp`]. -#[repr(C)] -#[derive(StableAbi)] +#[stabby::stabby] +#[repr(u8)] +#[expect(non_camel_case_types)] pub enum FFI_InsertOp { Append, Overwrite, diff --git a/datafusion/ffi/src/lib.rs b/datafusion/ffi/src/lib.rs index d7410e8483735..cde9d616cbc39 100644 --- a/datafusion/ffi/src/lib.rs +++ b/datafusion/ffi/src/lib.rs @@ -32,6 +32,8 @@ pub mod config; pub mod execution; pub mod execution_plan; pub mod expr; +pub mod ffi_future; +pub mod ffi_option; pub mod insert_op; pub mod physical_expr; pub mod plan_properties; diff --git a/datafusion/ffi/src/physical_expr/mod.rs b/datafusion/ffi/src/physical_expr/mod.rs index d268dd613f987..7fc1099002d81 100644 --- a/datafusion/ffi/src/physical_expr/mod.rs +++ b/datafusion/ffi/src/physical_expr/mod.rs @@ -24,8 +24,8 @@ use std::fmt::{Display, Formatter}; use std::hash::{DefaultHasher, Hash, Hasher}; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RResult, RString, RVec}; +use crate::ffi_option::FfiOption; +use crate::ffi_option::FfiResult; use arrow::array::{ArrayRef, BooleanArray, RecordBatch}; use arrow::datatypes::SchemaRef; use arrow_schema::ffi::FFI_ArrowSchema; @@ -37,6 +37,8 @@ use datafusion_expr::sort_properties::ExprProperties; use datafusion_expr::statistics::Distribution; use datafusion_physical_expr::PhysicalExpr; use datafusion_physical_expr_common::physical_expr::fmt_sql; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::{WrappedArray, WrappedSchema}; use crate::expr::columnar_value::FFI_ColumnarValue; @@ -50,7 +52,7 @@ use crate::util::FFIResult; use crate::{df_result, rresult, rresult_return}; #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_PhysicalExpr { pub data_type: unsafe extern "C" fn( &Self, @@ -74,50 +76,50 @@ pub struct FFI_PhysicalExpr { selection: WrappedArray, ) -> FFIResult, - pub children: unsafe extern "C" fn(&Self) -> RVec, + pub children: unsafe extern "C" fn(&Self) -> SVec, pub new_with_children: - unsafe extern "C" fn(&Self, children: &RVec) -> FFIResult, + unsafe extern "C" fn(&Self, children: &SVec) -> FFIResult, pub evaluate_bounds: unsafe extern "C" fn( &Self, - children: RVec, + children: SVec, ) -> FFIResult, pub propagate_constraints: unsafe extern "C" fn( &Self, interval: FFI_Interval, - children: RVec, - ) -> FFIResult>>, + children: SVec, + ) -> FFIResult>>, pub evaluate_statistics: unsafe extern "C" fn( &Self, - children: RVec, + children: SVec, ) -> FFIResult, pub propagate_statistics: unsafe extern "C" fn( &Self, parent: FFI_Distribution, - children: RVec, - ) -> FFIResult>>, + children: SVec, + ) -> FFIResult>>, pub get_properties: unsafe extern "C" fn( &Self, - children: RVec, + children: SVec, ) -> FFIResult, - pub fmt_sql: unsafe extern "C" fn(&Self) -> FFIResult, + pub fmt_sql: unsafe extern "C" fn(&Self) -> FFIResult, - pub snapshot: unsafe extern "C" fn(&Self) -> FFIResult>, + pub snapshot: unsafe extern "C" fn(&Self) -> FFIResult>, pub snapshot_generation: unsafe extern "C" fn(&Self) -> u64, pub is_volatile_node: unsafe extern "C" fn(&Self) -> bool, // Display trait - pub display: unsafe extern "C" fn(&Self) -> RString, + pub display: unsafe extern "C" fn(&Self) -> SString, // Hash trait pub hash: unsafe extern "C" fn(&Self) -> u64, @@ -226,7 +228,7 @@ unsafe extern "C" fn evaluate_selection_fn_wrapper( unsafe extern "C" fn children_fn_wrapper( expr: &FFI_PhysicalExpr, -) -> RVec { +) -> SVec { let expr = expr.inner(); let children = expr.children(); children @@ -237,7 +239,7 @@ unsafe extern "C" fn children_fn_wrapper( unsafe extern "C" fn new_with_children_fn_wrapper( expr: &FFI_PhysicalExpr, - children: &RVec, + children: &SVec, ) -> FFIResult { let expr = Arc::clone(expr.inner()); let children = children.iter().map(Into::into).collect::>(); @@ -246,7 +248,7 @@ unsafe extern "C" fn new_with_children_fn_wrapper( unsafe extern "C" fn evaluate_bounds_fn_wrapper( expr: &FFI_PhysicalExpr, - children: RVec, + children: SVec, ) -> FFIResult { let expr = expr.inner(); let children = rresult_return!( @@ -266,8 +268,8 @@ unsafe extern "C" fn evaluate_bounds_fn_wrapper( unsafe extern "C" fn propagate_constraints_fn_wrapper( expr: &FFI_PhysicalExpr, interval: FFI_Interval, - children: RVec, -) -> FFIResult>> { + children: SVec, +) -> FFIResult>> { let expr = expr.inner(); let interval = rresult_return!(Interval::try_from(interval)); let children = rresult_return!( @@ -286,16 +288,16 @@ unsafe extern "C" fn propagate_constraints_fn_wrapper( .map(|intervals| intervals .into_iter() .map(FFI_Interval::try_from) - .collect::>>()) + .collect::>>()) .transpose() ); - RResult::ROk(result.into()) + FfiResult::Ok(result.into()) } unsafe extern "C" fn evaluate_statistics_fn_wrapper( expr: &FFI_PhysicalExpr, - children: RVec, + children: SVec, ) -> FFIResult { let expr = expr.inner(); let children = rresult_return!( @@ -314,8 +316,8 @@ unsafe extern "C" fn evaluate_statistics_fn_wrapper( unsafe extern "C" fn propagate_statistics_fn_wrapper( expr: &FFI_PhysicalExpr, parent: FFI_Distribution, - children: RVec, -) -> FFIResult>> { + children: SVec, +) -> FFIResult>> { let expr = expr.inner(); let parent = rresult_return!(Distribution::try_from(parent)); let children = rresult_return!( @@ -332,16 +334,16 @@ unsafe extern "C" fn propagate_statistics_fn_wrapper( .map(|dists| dists .iter() .map(FFI_Distribution::try_from) - .collect::>>()) + .collect::>>()) .transpose() ); - RResult::ROk(result.into()) + FfiResult::Ok(result.into()) } unsafe extern "C" fn get_properties_fn_wrapper( expr: &FFI_PhysicalExpr, - children: RVec, + children: SVec, ) -> FFIResult { let expr = expr.inner(); let children = rresult_return!( @@ -356,15 +358,15 @@ unsafe extern "C" fn get_properties_fn_wrapper( ) } -unsafe extern "C" fn fmt_sql_fn_wrapper(expr: &FFI_PhysicalExpr) -> FFIResult { +unsafe extern "C" fn fmt_sql_fn_wrapper(expr: &FFI_PhysicalExpr) -> FFIResult { let expr = expr.inner(); let result = fmt_sql(expr.as_ref()).to_string(); - RResult::ROk(result.into()) + FfiResult::Ok(SString::from(result.as_str())) } unsafe extern "C" fn snapshot_fn_wrapper( expr: &FFI_PhysicalExpr, -) -> FFIResult> { +) -> FFIResult> { let expr = expr.inner(); rresult!( expr.snapshot() @@ -381,9 +383,9 @@ unsafe extern "C" fn is_volatile_node_fn_wrapper(expr: &FFI_PhysicalExpr) -> boo let expr = expr.inner(); expr.is_volatile_node() } -unsafe extern "C" fn display_fn_wrapper(expr: &FFI_PhysicalExpr) -> RString { +unsafe extern "C" fn display_fn_wrapper(expr: &FFI_PhysicalExpr) -> SString { let expr = expr.inner(); - format!("{expr}").into() + SString::from(format!("{expr}").as_str()) } unsafe extern "C" fn hash_fn_wrapper(expr: &FFI_PhysicalExpr) -> u64 { @@ -581,11 +583,9 @@ impl PhysicalExpr for ForeignPhysicalExpr { unsafe { let children = children.into_iter().map(FFI_PhysicalExpr::from).collect(); df_result!( - (self.expr.new_with_children)(&self.expr, &children).map(|expr| >::from( - &expr - )) + (self.expr.new_with_children)(&self.expr, &children) + .into_result() + .map(|expr| >::from(&expr)) ) } } @@ -595,7 +595,7 @@ impl PhysicalExpr for ForeignPhysicalExpr { let children = children .iter() .map(|interval| FFI_Interval::try_from(*interval)) - .collect::>>()?; + .collect::>>()?; df_result!((self.expr.evaluate_bounds)(&self.expr, children)) .and_then(Interval::try_from) } @@ -611,19 +611,18 @@ impl PhysicalExpr for ForeignPhysicalExpr { let children = children .iter() .map(|interval| FFI_Interval::try_from(*interval)) - .collect::>>()?; + .collect::>>()?; let result = df_result!((self.expr.propagate_constraints)( &self.expr, interval, children ))?; - let result: Option<_> = result - .map(|intervals| { - intervals - .into_iter() - .map(Interval::try_from) - .collect::>>() - }) - .into(); + let result: Option> = result.into(); + let result = result.map(|intervals| { + intervals + .into_iter() + .map(Interval::try_from) + .collect::>>() + }); result.transpose() } } @@ -633,7 +632,7 @@ impl PhysicalExpr for ForeignPhysicalExpr { let children = children .iter() .map(|dist| FFI_Distribution::try_from(*dist)) - .collect::>>()?; + .collect::>>()?; let result = df_result!((self.expr.evaluate_statistics)(&self.expr, children))?; @@ -651,19 +650,18 @@ impl PhysicalExpr for ForeignPhysicalExpr { let children = children .iter() .map(|dist| FFI_Distribution::try_from(*dist)) - .collect::>>()?; + .collect::>>()?; let result = df_result!((self.expr.propagate_statistics)( &self.expr, parent, children ))?; - let result: Option>> = result - .map(|dists| { - dists - .into_iter() - .map(Distribution::try_from) - .collect::>>() - }) - .into(); + let result: Option> = result.into(); + let result: Option>> = result.map(|dists| { + dists + .into_iter() + .map(Distribution::try_from) + .collect::>>() + }); result.transpose() } @@ -674,7 +672,7 @@ impl PhysicalExpr for ForeignPhysicalExpr { let children = children .iter() .map(FFI_ExprProperties::try_from) - .collect::>>()?; + .collect::>>()?; df_result!((self.expr.get_properties)(&self.expr, children)) .and_then(ExprProperties::try_from) } @@ -682,9 +680,10 @@ impl PhysicalExpr for ForeignPhysicalExpr { fn fmt_sql(&self, f: &mut Formatter<'_>) -> std::fmt::Result { unsafe { - match (self.expr.fmt_sql)(&self.expr) { - RResult::ROk(sql) => write!(f, "{sql}"), - RResult::RErr(_) => Err(std::fmt::Error), + match Into::>::into((self.expr.fmt_sql)(&self.expr)) + { + Ok(sql) => write!(f, "{sql}"), + Err(_) => Err(std::fmt::Error), } } } @@ -692,9 +691,8 @@ impl PhysicalExpr for ForeignPhysicalExpr { fn snapshot(&self) -> Result>> { unsafe { let result = df_result!((self.expr.snapshot)(&self.expr))?; - Ok(result - .map(|expr| >::from(&expr)) - .into()) + let result: Option = result.into(); + Ok(result.map(|expr| >::from(&expr))) } } diff --git a/datafusion/ffi/src/physical_expr/partitioning.rs b/datafusion/ffi/src/physical_expr/partitioning.rs index cda4fd2c97f45..c4f538a363cc5 100644 --- a/datafusion/ffi/src/physical_expr/partitioning.rs +++ b/datafusion/ffi/src/physical_expr/partitioning.rs @@ -17,20 +17,19 @@ use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::RVec; use datafusion_physical_expr::Partitioning; use datafusion_physical_expr_common::physical_expr::PhysicalExpr; +use stabby::alloc::vec::Vec as SVec; use crate::physical_expr::FFI_PhysicalExpr; /// A stable struct for sharing [`Partitioning`] across FFI boundaries. /// See ['Partitioning'] for the meaning of each variant. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub enum FFI_Partitioning { RoundRobinBatch(usize), - Hash(RVec, usize), + Hash(SVec, usize), UnknownPartitioning(usize), } diff --git a/datafusion/ffi/src/physical_expr/sort.rs b/datafusion/ffi/src/physical_expr/sort.rs index fd3339b10555a..fc8e2a81f36eb 100644 --- a/datafusion/ffi/src/physical_expr/sort.rs +++ b/datafusion/ffi/src/physical_expr/sort.rs @@ -17,7 +17,6 @@ use std::sync::Arc; -use abi_stable::StableAbi; use arrow_schema::SortOptions; use datafusion_physical_expr::PhysicalSortExpr; use datafusion_physical_expr_common::physical_expr::PhysicalExpr; @@ -28,7 +27,7 @@ use crate::physical_expr::FFI_PhysicalExpr; /// A stable struct for sharing [`PhysicalSortExpr`] across FFI boundaries. /// See [`PhysicalSortExpr`] for the meaning of each field. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_PhysicalSortExpr { expr: FFI_PhysicalExpr, options: FFI_SortOptions, diff --git a/datafusion/ffi/src/plan_properties.rs b/datafusion/ffi/src/plan_properties.rs index d009de3f04b99..4be35af584623 100644 --- a/datafusion/ffi/src/plan_properties.rs +++ b/datafusion/ffi/src/plan_properties.rs @@ -18,14 +18,14 @@ use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RVec}; +use crate::ffi_option::FfiOption; use arrow::datatypes::SchemaRef; use datafusion_common::error::{DataFusionError, Result}; use datafusion_physical_expr::EquivalenceProperties; use datafusion_physical_expr_common::sort_expr::PhysicalSortExpr; use datafusion_physical_plan::PlanProperties; use datafusion_physical_plan::execution_plan::{Boundedness, EmissionType}; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::WrappedSchema; use crate::physical_expr::partitioning::FFI_Partitioning; @@ -33,7 +33,7 @@ use crate::physical_expr::sort::FFI_PhysicalSortExpr; /// A stable struct for sharing [`PlanProperties`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_PlanProperties { /// The output partitioning of the plan. pub output_partitioning: unsafe extern "C" fn(plan: &Self) -> FFI_Partitioning, @@ -46,7 +46,7 @@ pub struct FFI_PlanProperties { /// The output ordering of the plan. pub output_ordering: - unsafe extern "C" fn(plan: &Self) -> ROption>, + unsafe extern "C" fn(plan: &Self) -> FfiOption>, /// Return the schema of the plan. pub schema: unsafe extern "C" fn(plan: &Self) -> WrappedSchema, @@ -95,8 +95,8 @@ unsafe extern "C" fn boundedness_fn_wrapper( unsafe extern "C" fn output_ordering_fn_wrapper( properties: &FFI_PlanProperties, -) -> ROption> { - let ordering: Option> = +) -> FfiOption> { + let ordering: Option> = properties.inner().output_ordering().map(|lex_ordering| { let vec_ordering: Vec = lex_ordering.clone().into(); vec_ordering @@ -159,7 +159,7 @@ impl TryFrom for PlanProperties { let ffi_schema = unsafe { (ffi_props.schema)(&ffi_props) }; let schema = (&ffi_schema.0).try_into()?; - let ffi_orderings: Option> = + let ffi_orderings: Option> = unsafe { (ffi_props.output_ordering)(&ffi_props) }.into(); let sort_exprs = ffi_orderings .map(|ordering_vec| { @@ -195,7 +195,7 @@ impl TryFrom for PlanProperties { /// FFI safe version of [`Boundedness`]. #[repr(C)] -#[derive(Clone, StableAbi)] +#[derive(Clone)] pub enum FFI_Boundedness { Bounded, Unbounded { requires_infinite_memory: bool }, @@ -229,7 +229,7 @@ impl From for Boundedness { /// FFI safe version of [`EmissionType`]. #[repr(C)] -#[derive(Clone, StableAbi)] +#[derive(Clone)] pub enum FFI_EmissionType { Incremental, Final, diff --git a/datafusion/ffi/src/proto/logical_extension_codec.rs b/datafusion/ffi/src/proto/logical_extension_codec.rs index 3781a40539ed1..4f5f1010f2408 100644 --- a/datafusion/ffi/src/proto/logical_extension_codec.rs +++ b/datafusion/ffi/src/proto/logical_extension_codec.rs @@ -18,8 +18,7 @@ use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{RResult, RSlice, RStr, RVec}; +use crate::ffi_option::FfiResult; use arrow::datatypes::SchemaRef; use datafusion_catalog::TableProvider; use datafusion_common::error::Result; @@ -33,6 +32,9 @@ use datafusion_expr::{ use datafusion_proto::logical_plan::{ DefaultLogicalExtensionCodec, LogicalExtensionCodec, }; +use stabby::alloc::vec::Vec as SVec; +use stabby::slice::Slice as SSlice; +use stabby::str::Str as SStr; use tokio::runtime::Handle; use crate::arrow_wrappers::WrappedSchema; @@ -46,55 +48,55 @@ use crate::{df_result, rresult_return}; /// A stable struct for sharing [`LogicalExtensionCodec`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_LogicalExtensionCodec { /// Decode bytes into a table provider. try_decode_table_provider: unsafe extern "C" fn( &Self, - buf: RSlice, - table_ref: RStr, + buf: SSlice, + table_ref: SStr, schema: WrappedSchema, ) -> FFIResult, /// Encode a table provider into bytes. try_encode_table_provider: unsafe extern "C" fn( &Self, - table_ref: RStr, + table_ref: SStr, node: FFI_TableProvider, - ) -> FFIResult>, + ) -> FFIResult>, /// Decode bytes into a user defined scalar function. try_decode_udf: unsafe extern "C" fn( &Self, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult, /// Encode a user defined scalar function into bytes. try_encode_udf: - unsafe extern "C" fn(&Self, node: FFI_ScalarUDF) -> FFIResult>, + unsafe extern "C" fn(&Self, node: FFI_ScalarUDF) -> FFIResult>, /// Decode bytes into a user defined aggregate function. try_decode_udaf: unsafe extern "C" fn( &Self, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult, /// Encode a user defined aggregate function into bytes. try_encode_udaf: - unsafe extern "C" fn(&Self, node: FFI_AggregateUDF) -> FFIResult>, + unsafe extern "C" fn(&Self, node: FFI_AggregateUDF) -> FFIResult>, /// Decode bytes into a user defined window function. try_decode_udwf: unsafe extern "C" fn( &Self, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult, /// Encode a user defined window function into bytes. try_encode_udwf: - unsafe extern "C" fn(&Self, node: FFI_WindowUDF) -> FFIResult>, + unsafe extern "C" fn(&Self, node: FFI_WindowUDF) -> FFIResult>, pub task_ctx_provider: FFI_TaskContextProvider, @@ -143,8 +145,8 @@ impl FFI_LogicalExtensionCodec { unsafe extern "C" fn try_decode_table_provider_fn_wrapper( codec: &FFI_LogicalExtensionCodec, - buf: RSlice, - table_ref: RStr, + buf: SSlice, + table_ref: SStr, schema: WrappedSchema, ) -> FFIResult { let ctx = rresult_return!(codec.task_ctx()); @@ -160,7 +162,7 @@ unsafe extern "C" fn try_decode_table_provider_fn_wrapper( ctx.as_ref() )); - RResult::ROk(FFI_TableProvider::new_with_ffi_codec( + FfiResult::Ok(FFI_TableProvider::new_with_ffi_codec( table_provider, true, runtime, @@ -170,9 +172,9 @@ unsafe extern "C" fn try_decode_table_provider_fn_wrapper( unsafe extern "C" fn try_encode_table_provider_fn_wrapper( codec: &FFI_LogicalExtensionCodec, - table_ref: RStr, + table_ref: SStr, node: FFI_TableProvider, -) -> FFIResult> { +) -> FFIResult> { let table_ref = TableReference::from(table_ref.as_str()); let table_provider: Arc = (&node).into(); let codec = codec.inner(); @@ -184,26 +186,26 @@ unsafe extern "C" fn try_encode_table_provider_fn_wrapper( &mut bytes )); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn try_decode_udf_fn_wrapper( codec: &FFI_LogicalExtensionCodec, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult { let codec = codec.inner(); let udf = rresult_return!(codec.try_decode_udf(name.as_str(), buf.as_ref())); let udf = FFI_ScalarUDF::from(udf); - RResult::ROk(udf) + FfiResult::Ok(udf) } unsafe extern "C" fn try_encode_udf_fn_wrapper( codec: &FFI_LogicalExtensionCodec, node: FFI_ScalarUDF, -) -> FFIResult> { +) -> FFIResult> { let codec = codec.inner(); let node: Arc = (&node).into(); let node = ScalarUDF::new_from_shared_impl(node); @@ -211,25 +213,25 @@ unsafe extern "C" fn try_encode_udf_fn_wrapper( let mut bytes = Vec::new(); rresult_return!(codec.try_encode_udf(&node, &mut bytes)); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn try_decode_udaf_fn_wrapper( codec: &FFI_LogicalExtensionCodec, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult { let codec_inner = codec.inner(); - let udaf = rresult_return!(codec_inner.try_decode_udaf(name.into(), buf.as_ref())); + let udaf = rresult_return!(codec_inner.try_decode_udaf(&name, buf.as_ref())); let udaf = FFI_AggregateUDF::from(udaf); - RResult::ROk(udaf) + FfiResult::Ok(udaf) } unsafe extern "C" fn try_encode_udaf_fn_wrapper( codec: &FFI_LogicalExtensionCodec, node: FFI_AggregateUDF, -) -> FFIResult> { +) -> FFIResult> { let codec = codec.inner(); let udaf: Arc = (&node).into(); let udaf = AggregateUDF::new_from_shared_impl(udaf); @@ -237,25 +239,25 @@ unsafe extern "C" fn try_encode_udaf_fn_wrapper( let mut bytes = Vec::new(); rresult_return!(codec.try_encode_udaf(&udaf, &mut bytes)); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn try_decode_udwf_fn_wrapper( codec: &FFI_LogicalExtensionCodec, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult { let codec = codec.inner(); - let udwf = rresult_return!(codec.try_decode_udwf(name.into(), buf.as_ref())); + let udwf = rresult_return!(codec.try_decode_udwf(&name, buf.as_ref())); let udwf = FFI_WindowUDF::from(udwf); - RResult::ROk(udwf) + FfiResult::Ok(udwf) } unsafe extern "C" fn try_encode_udwf_fn_wrapper( codec: &FFI_LogicalExtensionCodec, node: FFI_WindowUDF, -) -> FFIResult> { +) -> FFIResult> { let codec = codec.inner(); let udwf: Arc = (&node).into(); let udwf = WindowUDF::new_from_shared_impl(udwf); @@ -263,7 +265,7 @@ unsafe extern "C" fn try_encode_udwf_fn_wrapper( let mut bytes = Vec::new(); rresult_return!(codec.try_encode_udwf(&udwf, &mut bytes)); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn release_fn_wrapper(provider: &mut FFI_LogicalExtensionCodec) { diff --git a/datafusion/ffi/src/proto/physical_extension_codec.rs b/datafusion/ffi/src/proto/physical_extension_codec.rs index 0577e72366478..1d341cf6eaa1c 100644 --- a/datafusion/ffi/src/proto/physical_extension_codec.rs +++ b/datafusion/ffi/src/proto/physical_extension_codec.rs @@ -18,8 +18,7 @@ use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{RResult, RSlice, RStr, RVec}; +use crate::ffi_option::FfiResult; use datafusion_common::error::Result; use datafusion_execution::TaskContext; use datafusion_expr::{ @@ -27,6 +26,9 @@ use datafusion_expr::{ }; use datafusion_physical_plan::ExecutionPlan; use datafusion_proto::physical_plan::PhysicalExtensionCodec; +use stabby::alloc::vec::Vec as SVec; +use stabby::slice::Slice as SSlice; +use stabby::str::Str as SStr; use tokio::runtime::Handle; use crate::execution::FFI_TaskContextProvider; @@ -39,51 +41,51 @@ use crate::{df_result, rresult_return}; /// A stable struct for sharing [`PhysicalExtensionCodec`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_PhysicalExtensionCodec { /// Decode bytes into an execution plan. try_decode: unsafe extern "C" fn( &Self, - buf: RSlice, - inputs: RVec, + buf: SSlice, + inputs: SVec, ) -> FFIResult, /// Encode an execution plan into bytes. try_encode: - unsafe extern "C" fn(&Self, node: FFI_ExecutionPlan) -> FFIResult>, + unsafe extern "C" fn(&Self, node: FFI_ExecutionPlan) -> FFIResult>, /// Decode bytes into a user defined scalar function. try_decode_udf: unsafe extern "C" fn( &Self, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult, /// Encode a user defined scalar function into bytes. try_encode_udf: - unsafe extern "C" fn(&Self, node: FFI_ScalarUDF) -> FFIResult>, + unsafe extern "C" fn(&Self, node: FFI_ScalarUDF) -> FFIResult>, /// Decode bytes into a user defined aggregate function. try_decode_udaf: unsafe extern "C" fn( &Self, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult, /// Encode a user defined aggregate function into bytes. try_encode_udaf: - unsafe extern "C" fn(&Self, node: FFI_AggregateUDF) -> FFIResult>, + unsafe extern "C" fn(&Self, node: FFI_AggregateUDF) -> FFIResult>, /// Decode bytes into a user defined window function. try_decode_udwf: unsafe extern "C" fn( &Self, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult, /// Encode a user defined window function into bytes. try_encode_udwf: - unsafe extern "C" fn(&Self, node: FFI_WindowUDF) -> FFIResult>, + unsafe extern "C" fn(&Self, node: FFI_WindowUDF) -> FFIResult>, /// Access the current [`TaskContext`]. task_ctx_provider: FFI_TaskContextProvider, @@ -129,8 +131,8 @@ impl FFI_PhysicalExtensionCodec { unsafe extern "C" fn try_decode_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, - buf: RSlice, - inputs: RVec, + buf: SSlice, + inputs: SVec, ) -> FFIResult { let task_ctx: Arc = rresult_return!((&codec.task_ctx_provider).try_into()); @@ -144,13 +146,13 @@ unsafe extern "C" fn try_decode_fn_wrapper( let plan = rresult_return!(codec.try_decode(buf.as_ref(), &inputs, task_ctx.as_ref())); - RResult::ROk(FFI_ExecutionPlan::new(plan, None)) + FfiResult::Ok(FFI_ExecutionPlan::new(plan, None)) } unsafe extern "C" fn try_encode_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, node: FFI_ExecutionPlan, -) -> FFIResult> { +) -> FFIResult> { let codec = codec.inner(); let plan: Arc = rresult_return!((&node).try_into()); @@ -158,26 +160,26 @@ unsafe extern "C" fn try_encode_fn_wrapper( let mut bytes = Vec::new(); rresult_return!(codec.try_encode(plan, &mut bytes)); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn try_decode_udf_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult { let codec = codec.inner(); let udf = rresult_return!(codec.try_decode_udf(name.as_str(), buf.as_ref())); let udf = FFI_ScalarUDF::from(udf); - RResult::ROk(udf) + FfiResult::Ok(udf) } unsafe extern "C" fn try_encode_udf_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, node: FFI_ScalarUDF, -) -> FFIResult> { +) -> FFIResult> { let codec = codec.inner(); let node: Arc = (&node).into(); let node = ScalarUDF::new_from_shared_impl(node); @@ -185,25 +187,25 @@ unsafe extern "C" fn try_encode_udf_fn_wrapper( let mut bytes = Vec::new(); rresult_return!(codec.try_encode_udf(&node, &mut bytes)); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn try_decode_udaf_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult { let codec_inner = codec.inner(); - let udaf = rresult_return!(codec_inner.try_decode_udaf(name.into(), buf.as_ref())); + let udaf = rresult_return!(codec_inner.try_decode_udaf(&name, buf.as_ref())); let udaf = FFI_AggregateUDF::from(udaf); - RResult::ROk(udaf) + FfiResult::Ok(udaf) } unsafe extern "C" fn try_encode_udaf_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, node: FFI_AggregateUDF, -) -> FFIResult> { +) -> FFIResult> { let codec = codec.inner(); let udaf: Arc = (&node).into(); let udaf = AggregateUDF::new_from_shared_impl(udaf); @@ -211,25 +213,25 @@ unsafe extern "C" fn try_encode_udaf_fn_wrapper( let mut bytes = Vec::new(); rresult_return!(codec.try_encode_udaf(&udaf, &mut bytes)); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn try_decode_udwf_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, - name: RStr, - buf: RSlice, + name: SStr, + buf: SSlice, ) -> FFIResult { let codec = codec.inner(); - let udwf = rresult_return!(codec.try_decode_udwf(name.into(), buf.as_ref())); + let udwf = rresult_return!(codec.try_decode_udwf(&name, buf.as_ref())); let udwf = FFI_WindowUDF::from(udwf); - RResult::ROk(udwf) + FfiResult::Ok(udwf) } unsafe extern "C" fn try_encode_udwf_fn_wrapper( codec: &FFI_PhysicalExtensionCodec, node: FFI_WindowUDF, -) -> FFIResult> { +) -> FFIResult> { let codec = codec.inner(); let udwf: Arc = (&node).into(); let udwf = WindowUDF::new_from_shared_impl(udwf); @@ -237,7 +239,7 @@ unsafe extern "C" fn try_encode_udwf_fn_wrapper( let mut bytes = Vec::new(); rresult_return!(codec.try_encode_udwf(&udwf, &mut bytes)); - RResult::ROk(bytes.into()) + FfiResult::Ok(bytes.into_iter().collect()) } unsafe extern "C" fn release_fn_wrapper(provider: &mut FFI_PhysicalExtensionCodec) { diff --git a/datafusion/ffi/src/record_batch_stream.rs b/datafusion/ffi/src/record_batch_stream.rs index 53078a0e4bbae..7e3004451c361 100644 --- a/datafusion/ffi/src/record_batch_stream.rs +++ b/datafusion/ffi/src/record_batch_stream.rs @@ -18,11 +18,13 @@ use std::ffi::c_void; use std::task::Poll; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RResult}; +use crate::ffi_option::FfiOption; +use crate::ffi_option::FfiResult; + use arrow::array::{Array, RecordBatch, StructArray, make_array}; use arrow::ffi::{from_ffi, to_ffi}; -use async_ffi::{ContextExt, FfiContext, FfiPoll}; + +use crate::ffi_future::{ContextExt, FfiContext, FfiPoll}; use datafusion_common::{DataFusionError, Result, ffi_datafusion_err, ffi_err}; use datafusion_execution::{RecordBatchStream, SendableRecordBatchStream}; use futures::{Stream, TryStreamExt}; @@ -33,16 +35,16 @@ use crate::rresult; use crate::util::FFIResult; /// A stable struct for sharing [`RecordBatchStream`] across FFI boundaries. -/// We use the async-ffi crate for handling async calls across libraries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_RecordBatchStream { /// This mirrors the `poll_next` of [`RecordBatchStream`] but does so /// in a FFI safe manner. pub poll_next: unsafe extern "C" fn( stream: &Self, cx: &mut FfiContext, - ) -> FfiPoll>>, + ) + -> FfiPoll>>, /// Return the schema of the record batch pub schema: unsafe extern "C" fn(stream: &Self) -> WrappedSchema, @@ -116,20 +118,22 @@ pub(crate) fn record_batch_to_wrapped_array( // probably want to use pub unsafe fn from_ffi(array: FFI_ArrowArray, schema: &FFI_ArrowSchema) -> Result { fn maybe_record_batch_to_wrapped_stream( record_batch: Option>, -) -> ROption> { +) -> FfiOption> { match record_batch { Some(Ok(record_batch)) => { - ROption::RSome(record_batch_to_wrapped_array(record_batch)) + FfiOption::Some(record_batch_to_wrapped_array(record_batch)) } - Some(Err(e)) => ROption::RSome(RResult::RErr(e.to_string().into())), - None => ROption::RNone, + Some(Err(e)) => FfiOption::Some(FfiResult::Err( + stabby::alloc::string::String::from(e.to_string().as_str()), + )), + None => FfiOption::None, } } unsafe extern "C" fn poll_next_fn_wrapper( stream: &FFI_RecordBatchStream, cx: &mut FfiContext, -) -> FfiPoll>> { +) -> FfiPoll>> { unsafe { let private_data = stream.private_data as *mut RecordBatchStreamPrivateData; let stream = &mut (*private_data).rbs; @@ -142,7 +146,10 @@ unsafe extern "C" fn poll_next_fn_wrapper( .map(maybe_record_batch_to_wrapped_stream) }); - poll_result.into() + match poll_result { + Poll::Ready(v) => FfiPoll::Ready(v), + Poll::Pending => FfiPoll::Pending, + } } } @@ -171,14 +178,18 @@ pub(crate) fn wrapped_array_to_record_batch(array: WrappedArray) -> Result>, + array: FfiOption>, ) -> Option> { + let array: Option> = array.into(); match array { - ROption::RSome(RResult::ROk(wrapped_array)) => { - Some(wrapped_array_to_record_batch(wrapped_array)) + Some(result) => { + let result: std::result::Result = result.into(); + match result { + Ok(wrapped_array) => Some(wrapped_array_to_record_batch(wrapped_array)), + Err(e) => Some(ffi_err!("{e}")), + } } - ROption::RSome(RResult::RErr(e)) => Some(ffi_err!("{e}")), - ROption::RNone => None, + None => None, } } diff --git a/datafusion/ffi/src/schema_provider.rs b/datafusion/ffi/src/schema_provider.rs index b8e44b134f87b..f79cbfe3ce563 100644 --- a/datafusion/ffi/src/schema_provider.rs +++ b/datafusion/ffi/src/schema_provider.rs @@ -19,9 +19,12 @@ use std::any::Any; use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RResult, RString, RVec}; -use async_ffi::{FfiFuture, FutureExt}; +use crate::ffi_option::FfiOption; +use crate::ffi_option::FfiResult; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; + +use crate::ffi_future::{FfiFuture, FutureExt}; use async_trait::async_trait; use datafusion_catalog::{SchemaProvider, TableProvider}; use datafusion_common::error::{DataFusionError, Result}; @@ -38,32 +41,32 @@ use crate::{df_result, rresult_return}; /// A stable struct for sharing [`SchemaProvider`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_SchemaProvider { - pub owner_name: ROption, + pub owner_name: FfiOption, - pub table_names: unsafe extern "C" fn(provider: &Self) -> RVec, + pub table_names: unsafe extern "C" fn(provider: &Self) -> SVec, pub table: unsafe extern "C" fn( provider: &Self, - name: RString, + name: SString, ) - -> FfiFuture>>, + -> FfiFuture>>, pub register_table: unsafe extern "C" fn( provider: &Self, - name: RString, + name: SString, table: FFI_TableProvider, ) - -> FFIResult>, + -> FFIResult>, pub deregister_table: unsafe extern "C" fn( provider: &Self, - name: RString, + name: SString, ) - -> FFIResult>, + -> FFIResult>, - pub table_exist: unsafe extern "C" fn(provider: &Self, name: RString) -> bool, + pub table_exist: unsafe extern "C" fn(provider: &Self, name: SString) -> bool, pub logical_codec: FFI_LogicalExtensionCodec, @@ -113,19 +116,22 @@ impl FFI_SchemaProvider { unsafe extern "C" fn table_names_fn_wrapper( provider: &FFI_SchemaProvider, -) -> RVec { +) -> SVec { unsafe { let provider = provider.inner(); let table_names = provider.table_names(); - table_names.into_iter().map(|s| s.into()).collect() + table_names + .into_iter() + .map(|s| SString::from(s.as_str())) + .collect() } } unsafe extern "C" fn table_fn_wrapper( provider: &FFI_SchemaProvider, - name: RString, -) -> FfiFuture>> { + name: SString, +) -> FfiFuture>> { unsafe { let runtime = provider.runtime(); let logical_codec = provider.logical_codec.clone(); @@ -138,7 +144,7 @@ unsafe extern "C" fn table_fn_wrapper( }) .into(); - RResult::ROk(table) + FfiResult::Ok(table) } .into_ffi() } @@ -146,9 +152,9 @@ unsafe extern "C" fn table_fn_wrapper( unsafe extern "C" fn register_table_fn_wrapper( provider: &FFI_SchemaProvider, - name: RString, + name: SString, table: FFI_TableProvider, -) -> FFIResult> { +) -> FFIResult> { unsafe { let runtime = provider.runtime(); let logical_codec = provider.logical_codec.clone(); @@ -156,19 +162,19 @@ unsafe extern "C" fn register_table_fn_wrapper( let table = Arc::new(ForeignTableProvider(table)); - let returned_table = rresult_return!(provider.register_table(name.into(), table)) - .map(|t| { + let returned_table = + rresult_return!(provider.register_table(name.to_string(), table)).map(|t| { FFI_TableProvider::new_with_ffi_codec(t, true, runtime, logical_codec) }); - RResult::ROk(returned_table.into()) + FfiResult::Ok(returned_table.into()) } } unsafe extern "C" fn deregister_table_fn_wrapper( provider: &FFI_SchemaProvider, - name: RString, -) -> FFIResult> { + name: SString, +) -> FFIResult> { unsafe { let runtime = provider.runtime(); let logical_codec = provider.logical_codec.clone(); @@ -179,13 +185,13 @@ unsafe extern "C" fn deregister_table_fn_wrapper( FFI_TableProvider::new_with_ffi_codec(t, true, runtime, logical_codec) }); - RResult::ROk(returned_table.into()) + FfiResult::Ok(returned_table.into()) } } unsafe extern "C" fn table_exist_fn_wrapper( provider: &FFI_SchemaProvider, - name: RString, + name: SString, ) -> bool { unsafe { provider.inner().table_exist(name.as_str()) } } @@ -259,7 +265,7 @@ impl FFI_SchemaProvider { runtime: Option, logical_codec: FFI_LogicalExtensionCodec, ) -> Self { - let owner_name = provider.owner_name().map(|s| s.into()).into(); + let owner_name = provider.owner_name().map(SString::from).into(); let private_data = Box::new(ProviderPrivateData { provider, runtime }); Self { @@ -313,15 +319,14 @@ impl SchemaProvider for ForeignSchemaProvider { } fn owner_name(&self) -> Option<&str> { - let name: Option<&RString> = self.0.owner_name.as_ref().into(); - name.map(|s| s.as_str()) + self.0.owner_name.as_ref().map(|s| s.as_str()) } fn table_names(&self) -> Vec { unsafe { (self.0.table_names)(&self.0) .into_iter() - .map(|s| s.into()) + .map(|s| s.to_string()) .collect() } } @@ -332,7 +337,7 @@ impl SchemaProvider for ForeignSchemaProvider { ) -> Result>, DataFusionError> { unsafe { let table: Option = - df_result!((self.0.table)(&self.0, name.into()).await)?.into(); + df_result!((self.0.table)(&self.0, SString::from(name)).await)?.into(); let table = table.as_ref().map(>::from); @@ -357,8 +362,12 @@ impl SchemaProvider for ForeignSchemaProvider { }; let returned_provider: Option = - df_result!((self.0.register_table)(&self.0, name.into(), ffi_table))? - .into(); + df_result!((self.0.register_table)( + &self.0, + SString::from(name.as_str()), + ffi_table + ))? + .into(); Ok(returned_provider .map(|t| Arc::new(ForeignTableProvider(t)) as Arc)) @@ -367,7 +376,7 @@ impl SchemaProvider for ForeignSchemaProvider { fn deregister_table(&self, name: &str) -> Result>> { let returned_provider: Option = unsafe { - df_result!((self.0.deregister_table)(&self.0, name.into()))?.into() + df_result!((self.0.deregister_table)(&self.0, SString::from(name)))?.into() }; Ok(returned_provider @@ -376,7 +385,7 @@ impl SchemaProvider for ForeignSchemaProvider { /// Returns true if table exist in the schema provider, false otherwise. fn table_exist(&self, name: &str) -> bool { - unsafe { (self.0.table_exist)(&self.0, name.into()) } + unsafe { (self.0.table_exist)(&self.0, SString::from(name)) } } } diff --git a/datafusion/ffi/src/session/config.rs b/datafusion/ffi/src/session/config.rs index 63f0f20ecc7d5..fca0190c07138 100644 --- a/datafusion/ffi/src/session/config.rs +++ b/datafusion/ffi/src/session/config.rs @@ -18,7 +18,6 @@ use std::ffi::c_void; use crate::config::FFI_ConfigOptions; -use abi_stable::StableAbi; use datafusion_common::config::ConfigOptions; use datafusion_common::error::{DataFusionError, Result}; use datafusion_execution::config::SessionConfig; @@ -35,7 +34,7 @@ use datafusion_execution::config::SessionConfig; /// SessionConfig via a FFI interface would be extensive and provide limited /// value over this version. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_SessionConfig { /// FFI stable configuration options. pub config_options: FFI_ConfigOptions, diff --git a/datafusion/ffi/src/session/mod.rs b/datafusion/ffi/src/session/mod.rs index 6b8664a437495..a24993013d17c 100644 --- a/datafusion/ffi/src/session/mod.rs +++ b/datafusion/ffi/src/session/mod.rs @@ -20,11 +20,15 @@ use std::collections::HashMap; use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{RHashMap, RResult, RStr, RString, RVec}; +use crate::ffi_option::FfiResult; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; +use stabby::str::Str as SStr; + use arrow_schema::SchemaRef; use arrow_schema::ffi::FFI_ArrowSchema; -use async_ffi::{FfiFuture, FutureExt}; + +use crate::ffi_future::{FfiFuture, FutureExt}; use async_trait::async_trait; use datafusion_common::config::{ConfigFileType, ConfigOptions, TableOptions}; use datafusion_common::{DFSchema, DataFusionError}; @@ -74,34 +78,33 @@ pub mod config; /// which has methods that require `&dyn Session`. For usage within this crate /// we know the [`Session`] lifetimes are valid. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub(crate) struct FFI_SessionRef { - session_id: unsafe extern "C" fn(&Self) -> RStr, + session_id: unsafe extern "C" fn(&Self) -> SStr, config: unsafe extern "C" fn(&Self) -> FFI_SessionConfig, create_physical_plan: unsafe extern "C" fn( &Self, - logical_plan_serialized: RVec, + logical_plan_serialized: SVec, ) -> FfiFuture>, create_physical_expr: unsafe extern "C" fn( &Self, - expr_serialized: RVec, + expr_serialized: SVec, schema: WrappedSchema, ) -> FFIResult, - scalar_functions: unsafe extern "C" fn(&Self) -> RHashMap, + scalar_functions: unsafe extern "C" fn(&Self) -> SVec<(SString, FFI_ScalarUDF)>, - aggregate_functions: - unsafe extern "C" fn(&Self) -> RHashMap, + aggregate_functions: unsafe extern "C" fn(&Self) -> SVec<(SString, FFI_AggregateUDF)>, - window_functions: unsafe extern "C" fn(&Self) -> RHashMap, + window_functions: unsafe extern "C" fn(&Self) -> SVec<(SString, FFI_WindowUDF)>, - table_options: unsafe extern "C" fn(&Self) -> RHashMap, + table_options: unsafe extern "C" fn(&Self) -> SVec<(SString, SString)>, - default_table_options: unsafe extern "C" fn(&Self) -> RHashMap, + default_table_options: unsafe extern "C" fn(&Self) -> SVec<(SString, SString)>, task_ctx: unsafe extern "C" fn(&Self) -> FFI_TaskContext, @@ -148,7 +151,7 @@ impl FFI_SessionRef { } } -unsafe extern "C" fn session_id_fn_wrapper(session: &FFI_SessionRef) -> RStr<'_> { +unsafe extern "C" fn session_id_fn_wrapper(session: &FFI_SessionRef) -> SStr<'_> { let session = session.inner(); session.session_id().into() } @@ -160,7 +163,7 @@ unsafe extern "C" fn config_fn_wrapper(session: &FFI_SessionRef) -> FFI_SessionC unsafe extern "C" fn create_physical_plan_fn_wrapper( session: &FFI_SessionRef, - logical_plan_serialized: RVec, + logical_plan_serialized: SVec, ) -> FfiFuture> { unsafe { let runtime = session.runtime().clone(); @@ -184,7 +187,7 @@ unsafe extern "C" fn create_physical_plan_fn_wrapper( unsafe extern "C" fn create_physical_expr_fn_wrapper( session: &FFI_SessionRef, - expr_serialized: RVec, + expr_serialized: SVec, schema: WrappedSchema, ) -> FFIResult { let codec: Arc = (&session.logical_codec).into(); @@ -199,30 +202,35 @@ unsafe extern "C" fn create_physical_expr_fn_wrapper( let physical_expr = rresult_return!(session.create_physical_expr(logical_expr, &schema)); - RResult::ROk(physical_expr.into()) + FfiResult::Ok(physical_expr.into()) } unsafe extern "C" fn scalar_functions_fn_wrapper( session: &FFI_SessionRef, -) -> RHashMap { +) -> SVec<(SString, FFI_ScalarUDF)> { let session = session.inner(); session .scalar_functions() .iter() - .map(|(name, udf)| (name.clone().into(), FFI_ScalarUDF::from(Arc::clone(udf)))) + .map(|(name, udf)| { + ( + SString::from(name.as_str()), + FFI_ScalarUDF::from(Arc::clone(udf)), + ) + }) .collect() } unsafe extern "C" fn aggregate_functions_fn_wrapper( session: &FFI_SessionRef, -) -> RHashMap { +) -> SVec<(SString, FFI_AggregateUDF)> { let session = session.inner(); session .aggregate_functions() .iter() .map(|(name, udaf)| { ( - name.clone().into(), + SString::from(name.as_str()), FFI_AggregateUDF::from(Arc::clone(udaf)), ) }) @@ -231,56 +239,64 @@ unsafe extern "C" fn aggregate_functions_fn_wrapper( unsafe extern "C" fn window_functions_fn_wrapper( session: &FFI_SessionRef, -) -> RHashMap { +) -> SVec<(SString, FFI_WindowUDF)> { let session = session.inner(); session .window_functions() .iter() - .map(|(name, udwf)| (name.clone().into(), FFI_WindowUDF::from(Arc::clone(udwf)))) + .map(|(name, udwf)| { + ( + SString::from(name.as_str()), + FFI_WindowUDF::from(Arc::clone(udwf)), + ) + }) .collect() } -fn table_options_to_rhash(mut options: TableOptions) -> RHashMap { +fn table_options_to_svec(mut options: TableOptions) -> SVec<(SString, SString)> { // It is important that we mutate options here and set current format // to None so that when we call `entries()` we get ALL format entries. // We will pass current_format as a special case and strip it on the // other side of the boundary. let current_format = options.current_format.take(); - let mut options: HashMap = options + let mut options: Vec<(SString, SString)> = options .entries() .into_iter() - .filter_map(|entry| entry.value.map(|v| (entry.key.into(), v.into()))) + .filter_map(|entry| { + entry + .value + .map(|v| (SString::from(entry.key.as_str()), SString::from(v.as_str()))) + }) .collect(); if let Some(current_format) = current_format { - options.insert( - "datafusion_ffi.table_current_format".into(), - match current_format { + options.push(( + SString::from("datafusion_ffi.table_current_format"), + SString::from(match current_format { ConfigFileType::JSON => "json", ConfigFileType::PARQUET => "parquet", ConfigFileType::CSV => "csv", - } - .into(), - ); + }), + )); } - options.into() + options.into_iter().collect() } unsafe extern "C" fn table_options_fn_wrapper( session: &FFI_SessionRef, -) -> RHashMap { +) -> SVec<(SString, SString)> { let session = session.inner(); let table_options = session.table_options(); - table_options_to_rhash(table_options.clone()) + table_options_to_svec(table_options.clone()) } unsafe extern "C" fn default_table_options_fn_wrapper( session: &FFI_SessionRef, -) -> RHashMap { +) -> SVec<(SString, SString)> { let session = session.inner(); let table_options = session.default_table_options(); - table_options_to_rhash(table_options) + table_options_to_svec(table_options) } unsafe extern "C" fn task_ctx_fn_wrapper(session: &FFI_SessionRef) -> FFI_TaskContext { @@ -395,41 +411,40 @@ impl TryFrom<&FFI_SessionRef> for ForeignSession { type Error = DataFusionError; fn try_from(session: &FFI_SessionRef) -> Result { unsafe { - let table_options = - table_options_from_rhashmap((session.table_options)(session)); + let table_options = table_options_from_svec((session.table_options)(session)); let config = (session.config)(session); let config = SessionConfig::try_from(&config)?; let scalar_functions = (session.scalar_functions)(session) .into_iter() - .map(|kv_pair| { - let udf = >::from(&kv_pair.1); + .map(|(name, ffi_udf)| { + let udf = >::from(&ffi_udf); ( - kv_pair.0.into_string(), + name.to_string(), Arc::new(ScalarUDF::new_from_shared_impl(udf)), ) }) .collect(); let aggregate_functions = (session.aggregate_functions)(session) .into_iter() - .map(|kv_pair| { - let udaf = >::from(&kv_pair.1); + .map(|(name, ffi_udaf)| { + let udaf = >::from(&ffi_udaf); ( - kv_pair.0.into_string(), + name.to_string(), Arc::new(AggregateUDF::new_from_shared_impl(udaf)), ) }) .collect(); let window_functions = (session.window_functions)(session) .into_iter() - .map(|kv_pair| { - let udwf = >::from(&kv_pair.1); + .map(|(name, ffi_udwf)| { + let udwf = >::from(&ffi_udwf); ( - kv_pair.0.into_string(), + name.to_string(), Arc::new(WindowUDF::new_from_shared_impl(udwf)), ) }) @@ -455,10 +470,10 @@ impl Clone for FFI_SessionRef { } } -fn table_options_from_rhashmap(options: RHashMap) -> TableOptions { +fn table_options_from_svec(options: SVec<(SString, SString)>) -> TableOptions { let mut options: HashMap = options .into_iter() - .map(|kv_pair| (kv_pair.0.into_string(), kv_pair.1.into_string())) + .map(|(k, v)| (k.to_string(), v.to_string())) .collect(); let current_format = options.remove("datafusion_ffi.table_current_format"); @@ -545,7 +560,7 @@ impl Session for ForeignSession { let physical_plan = df_result!( (self.session.create_physical_plan)( &self.session, - logical_plan.as_ref().into() + logical_plan.as_ref().iter().copied().collect() ) .await )?; @@ -568,7 +583,7 @@ impl Session for ForeignSession { let physical_expr = df_result!((self.session.create_physical_expr)( &self.session, - logical_expr.into(), + logical_expr.into_iter().collect(), schema ))?; @@ -606,9 +621,7 @@ impl Session for ForeignSession { fn default_table_options(&self) -> TableOptions { unsafe { - table_options_from_rhashmap((self.session.default_table_options)( - &self.session, - )) + table_options_from_svec((self.session.default_table_options)(&self.session)) } } diff --git a/datafusion/ffi/src/table_provider.rs b/datafusion/ffi/src/table_provider.rs index 1559549e6362d..276cd7a02d2d3 100644 --- a/datafusion/ffi/src/table_provider.rs +++ b/datafusion/ffi/src/table_provider.rs @@ -19,10 +19,13 @@ use std::any::Any; use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RResult, RVec}; +use crate::ffi_option::FfiOption; +use crate::ffi_option::FfiResult; +use stabby::alloc::vec::Vec as SVec; + use arrow::datatypes::SchemaRef; -use async_ffi::{FfiFuture, FutureExt}; + +use crate::ffi_future::{FfiFuture, FutureExt}; use async_trait::async_trait; use datafusion_catalog::{Session, TableProvider}; use datafusion_common::error::{DataFusionError, Result}; @@ -89,7 +92,7 @@ use crate::{df_result, rresult_return}; /// It is important to be careful when expanding these functions to be certain which /// side of the interface each object refers to. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_TableProvider { /// Return the table schema schema: unsafe extern "C" fn(provider: &Self) -> WrappedSchema, @@ -108,9 +111,9 @@ pub struct FFI_TableProvider { scan: unsafe extern "C" fn( provider: &Self, session: FFI_SessionRef, - projections: ROption>, - filters_serialized: RVec, - limit: ROption, + projections: FfiOption>, + filters_serialized: SVec, + limit: FfiOption, ) -> FfiFuture>, /// Return the type of table. See [`TableType`] for options. @@ -122,8 +125,8 @@ pub struct FFI_TableProvider { supports_filters_pushdown: Option< unsafe extern "C" fn( provider: &FFI_TableProvider, - filters_serialized: RVec, - ) -> FFIResult>, + filters_serialized: SVec, + ) -> FFIResult>, >, insert_into: unsafe extern "C" fn( @@ -190,7 +193,7 @@ fn supports_filters_pushdown_internal( filters_serialized: &[u8], task_ctx: &Arc, codec: &dyn LogicalExtensionCodec, -) -> Result> { +) -> Result> { let filters = match filters_serialized.is_empty() { true => vec![], false => { @@ -202,7 +205,7 @@ fn supports_filters_pushdown_internal( }; let filters_borrowed: Vec<&Expr> = filters.iter().collect(); - let results: RVec<_> = provider + let results: SVec<_> = provider .supports_filters_pushdown(&filters_borrowed)? .iter() .map(|v| v.into()) @@ -213,8 +216,8 @@ fn supports_filters_pushdown_internal( unsafe extern "C" fn supports_filters_pushdown_fn_wrapper( provider: &FFI_TableProvider, - filters_serialized: RVec, -) -> FFIResult> { + filters_serialized: SVec, +) -> FFIResult> { let logical_codec: Arc = (&provider.logical_codec).into(); let task_ctx = rresult_return!(>::try_from( &provider.logical_codec.task_ctx_provider @@ -225,16 +228,16 @@ unsafe extern "C" fn supports_filters_pushdown_fn_wrapper( &task_ctx, logical_codec.as_ref(), ) - .map_err(|e| e.to_string().into()) + .map_err(|e| stabby::alloc::string::String::from(e.to_string().as_str())) .into() } unsafe extern "C" fn scan_fn_wrapper( provider: &FFI_TableProvider, session: FFI_SessionRef, - projections: ROption>, - filters_serialized: RVec, - limit: ROption, + projections: FfiOption>, + filters_serialized: SVec, + limit: FfiOption, ) -> FfiFuture> { let task_ctx: Result, DataFusionError> = (&provider.logical_codec.task_ctx_provider).try_into(); @@ -269,8 +272,10 @@ unsafe extern "C" fn scan_fn_wrapper( } }; - let projections: Option> = - projections.into_option().map(|p| p.into_iter().collect()); + let projections: Option> = { + let opt: Option> = projections.into(); + opt.map(|p| p.into_iter().collect()) + }; let plan = rresult_return!( internal_provider @@ -278,7 +283,7 @@ unsafe extern "C" fn scan_fn_wrapper( .await ); - RResult::ROk(FFI_ExecutionPlan::new(plan, runtime.clone())) + FfiResult::Ok(FFI_ExecutionPlan::new(plan, runtime.clone())) } .into_ffi() } @@ -315,7 +320,7 @@ unsafe extern "C" fn insert_into_fn_wrapper( .await ); - RResult::ROk(FFI_ExecutionPlan::new(plan, runtime.clone())) + FfiResult::Ok(FFI_ExecutionPlan::new(plan, runtime.clone())) } .into_ffi() } @@ -462,7 +467,7 @@ impl TableProvider for ForeignTableProvider { ) -> Result> { let session = FFI_SessionRef::new(session, None, self.0.logical_codec.clone()); - let projections: ROption> = projection + let projections: FfiOption> = projection .map(|p| p.iter().map(|v| v.to_owned()).collect()) .into(); @@ -470,7 +475,8 @@ impl TableProvider for ForeignTableProvider { let filter_list = LogicalExprList { expr: serialize_exprs(filters, codec.as_ref())?, }; - let filters_serialized = filter_list.encode_to_vec().into(); + let filters_serialized: SVec = + filter_list.encode_to_vec().into_iter().collect(); let plan = unsafe { let maybe_plan = (self.0.scan)( @@ -515,7 +521,10 @@ impl TableProvider for ForeignTableProvider { }; let serialized_filters = expr_list.encode_to_vec(); - let pushdowns = df_result!(pushdown_fn(&self.0, serialized_filters.into()))?; + let pushdowns = df_result!(pushdown_fn( + &self.0, + serialized_filters.into_iter().collect() + ))?; Ok(pushdowns.iter().map(|v| v.into()).collect()) } diff --git a/datafusion/ffi/src/table_provider_factory.rs b/datafusion/ffi/src/table_provider_factory.rs index 15789eeab0421..9a7b054e922a6 100644 --- a/datafusion/ffi/src/table_provider_factory.rs +++ b/datafusion/ffi/src/table_provider_factory.rs @@ -17,11 +17,11 @@ use std::{ffi::c_void, sync::Arc}; -use abi_stable::{ - StableAbi, - std_types::{RResult, RString, RVec}, -}; -use async_ffi::{FfiFuture, FutureExt}; +use crate::ffi_option::FfiResult; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; + +use crate::ffi_future::{FfiFuture, FutureExt}; use async_trait::async_trait; use datafusion_catalog::{Session, TableProvider, TableProviderFactory}; use datafusion_common::error::{DataFusionError, Result}; @@ -49,7 +49,7 @@ use crate::{df_result, rresult_return}; /// /// [`FFI_TableProvider`]: crate::table_provider::FFI_TableProvider #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_TableProviderFactory { /// Create a TableProvider with the given command. /// @@ -62,8 +62,8 @@ pub struct FFI_TableProviderFactory { create: unsafe extern "C" fn( factory: &Self, session: FFI_SessionRef, - cmd_serialized: RVec, - ) -> FfiFuture>, + cmd_serialized: SVec, + ) -> FfiFuture>, logical_codec: FFI_LogicalExtensionCodec, @@ -144,7 +144,7 @@ impl FFI_TableProviderFactory { fn deserialize_cmd( &self, - cmd_serialized: &RVec, + cmd_serialized: &SVec, ) -> Result { let task_ctx: Arc = (&self.logical_codec.task_ctx_provider).try_into()?; @@ -186,15 +186,15 @@ impl From<&FFI_TableProviderFactory> for Arc { unsafe extern "C" fn create_fn_wrapper( factory: &FFI_TableProviderFactory, session: FFI_SessionRef, - cmd_serialized: RVec, -) -> FfiFuture> { + cmd_serialized: SVec, +) -> FfiFuture> { let factory = factory.clone(); async move { let provider = rresult_return!( create_fn_wrapper_impl(factory, session, cmd_serialized).await ); - RResult::ROk(provider) + FfiResult::Ok(provider) } .into_ffi() } @@ -202,7 +202,7 @@ unsafe extern "C" fn create_fn_wrapper( async fn create_fn_wrapper_impl( factory: FFI_TableProviderFactory, session: FFI_SessionRef, - cmd_serialized: RVec, + cmd_serialized: SVec, ) -> Result { let runtime = factory.runtime().clone(); let ffi_logical_codec = factory.logical_codec.clone(); @@ -269,7 +269,7 @@ impl ForeignTableProviderFactory { fn serialize_cmd( &self, cmd: CreateExternalTable, - ) -> Result, DataFusionError> { + ) -> Result, DataFusionError> { let logical_codec: Arc = (&self.0.logical_codec).into(); @@ -280,7 +280,7 @@ impl ForeignTableProviderFactory { let mut buf: Vec = Vec::new(); plan.try_encode(&mut buf)?; - Ok(buf.into()) + Ok(buf.into_iter().collect()) } } diff --git a/datafusion/ffi/src/table_source.rs b/datafusion/ffi/src/table_source.rs index 2f17d9235a088..25b539e2981a3 100644 --- a/datafusion/ffi/src/table_source.rs +++ b/datafusion/ffi/src/table_source.rs @@ -15,12 +15,12 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; use datafusion_expr::{TableProviderFilterPushDown, TableType}; /// FFI safe version of [`TableProviderFilterPushDown`]. -#[repr(C)] -#[derive(StableAbi)] +#[stabby::stabby] +#[repr(u8)] +#[expect(non_camel_case_types)] pub enum FFI_TableProviderFilterPushDown { Unsupported, Inexact, @@ -56,8 +56,10 @@ impl From<&TableProviderFilterPushDown> for FFI_TableProviderFilterPushDown { } /// FFI safe version of [`TableType`]. -#[repr(C)] -#[derive(Debug, Clone, Copy, PartialEq, Eq, StableAbi)] +#[stabby::stabby] +#[repr(u8)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[expect(non_camel_case_types)] pub enum FFI_TableType { Base, View, diff --git a/datafusion/ffi/src/tests/mod.rs b/datafusion/ffi/src/tests/mod.rs index f594993e35e80..43a8b0f770c2b 100644 --- a/datafusion/ffi/src/tests/mod.rs +++ b/datafusion/ffi/src/tests/mod.rs @@ -17,12 +17,6 @@ use std::sync::Arc; -use abi_stable::library::{LibraryError, RootModule}; -use abi_stable::prefix_type::PrefixTypeTrait; -use abi_stable::sabi_types::VersionStrings; -use abi_stable::{ - StableAbi, declare_root_module_statics, export_root_module, package_version_strings, -}; use arrow::array::RecordBatch; use arrow_schema::{DataType, Field, Schema}; use async_provider::create_async_table_provider; @@ -54,12 +48,10 @@ mod table_provider_factory; mod udf_udaf_udwf; pub mod utils; -#[repr(C)] -#[derive(StableAbi)] -#[sabi(kind(Prefix(prefix_ref = ForeignLibraryModuleRef)))] /// This struct defines the module interfaces. It is to be shared by /// both the module loading program and library that implements the /// module. +#[repr(C)] pub struct ForeignLibraryModule { /// Construct an opinionated catalog provider pub create_catalog: @@ -103,17 +95,6 @@ pub struct ForeignLibraryModule { pub version: extern "C" fn() -> u64, } -impl RootModule for ForeignLibraryModuleRef { - declare_root_module_statics! {ForeignLibraryModuleRef} - const BASE_NAME: &'static str = "datafusion_ffi"; - const NAME: &'static str = "datafusion_ffi"; - const VERSION_STRINGS: VersionStrings = package_version_strings!(); - - fn initialization(self) -> Result { - Ok(self) - } -} - pub fn create_test_schema() -> Arc { Arc::new(Schema::new(vec![ Field::new("a", DataType::Int32, true), @@ -149,9 +130,9 @@ extern "C" fn construct_table_provider_factory( table_provider_factory::create(codec) } -#[export_root_module] /// This defines the entry point for using the module. -pub fn get_foreign_library_module() -> ForeignLibraryModuleRef { +#[unsafe(no_mangle)] +pub extern "C" fn datafusion_ffi_get_module() -> ForeignLibraryModule { ForeignLibraryModule { create_catalog: create_catalog_provider, create_catalog_list: create_catalog_provider_list, @@ -167,5 +148,4 @@ pub fn get_foreign_library_module() -> ForeignLibraryModuleRef { create_extension_options: config::create_extension_options, version: super::version, } - .leak_into_prefix() } diff --git a/datafusion/ffi/src/tests/utils.rs b/datafusion/ffi/src/tests/utils.rs index 9659a51f04b01..e1374c786266b 100644 --- a/datafusion/ffi/src/tests/utils.rs +++ b/datafusion/ffi/src/tests/utils.rs @@ -15,47 +15,63 @@ // specific language governing permissions and limitations // under the License. -use std::path::Path; +use std::path::{Path, PathBuf}; -use abi_stable::library::RootModule; use datafusion_common::{DataFusionError, Result}; -use crate::tests::ForeignLibraryModuleRef; +use crate::tests::ForeignLibraryModule; -/// Compute the path to the library. It would be preferable to simply use -/// abi_stable::library::development_utils::compute_library_path however -/// our current CI pipeline has a `ci` profile that we need to use to -/// find the library. -pub fn compute_library_path( - target_path: &Path, -) -> std::io::Result { +/// Compute the path to the built cdylib. Checks debug, release, and ci profile dirs. +fn compute_library_dir(target_path: &Path) -> PathBuf { let debug_dir = target_path.join("debug"); let release_dir = target_path.join("release"); let ci_dir = target_path.join("ci"); - let debug_path = M::get_library_path(&debug_dir.join("deps")); - let release_path = M::get_library_path(&release_dir.join("deps")); - let ci_path = M::get_library_path(&ci_dir.join("deps")); + let all_dirs = vec![debug_dir.clone(), release_dir, ci_dir]; - let all_paths = vec![ - (debug_dir.clone(), debug_path), - (release_dir, release_path), - (ci_dir, ci_path), - ]; - - let best_path = all_paths + all_dirs .into_iter() - .filter(|(_, path)| path.exists()) - .filter_map(|(dir, path)| path.metadata().map(|m| (dir, m)).ok()) - .filter_map(|(dir, meta)| meta.modified().map(|m| (dir, m)).ok()) + .filter(|dir| dir.join("deps").exists()) + .filter_map(|dir| { + dir.join("deps") + .metadata() + .and_then(|m| m.modified()) + .ok() + .map(|date| (dir, date)) + }) .max_by_key(|(_, date)| *date) .map(|(dir, _)| dir) - .unwrap_or(debug_dir); + .unwrap_or(debug_dir) +} + +/// Find the cdylib file for datafusion_ffi in the given directory. +fn find_cdylib(deps_dir: &Path) -> Result { + let lib_prefix = if cfg!(target_os = "windows") { + "" + } else { + "lib" + }; + let lib_ext = if cfg!(target_os = "macos") { + "dylib" + } else if cfg!(target_os = "windows") { + "dll" + } else { + "so" + }; - Ok(best_path) + let pattern = format!("{lib_prefix}datafusion_ffi.{lib_ext}"); + let lib_path = deps_dir.join(&pattern); + + if lib_path.exists() { + return Ok(lib_path); + } + + Err(DataFusionError::External( + format!("Could not find library at {}", lib_path.display()).into(), + )) } -pub fn get_module() -> Result { +pub fn get_module() -> Result { let expected_version = crate::version(); let crate_root = Path::new(env!("CARGO_MANIFEST_DIR")); @@ -66,24 +82,26 @@ pub fn get_module() -> Result { .expect("Failed to find workspace root") .join("target"); - // Find the location of the library. This is specific to the build environment, - // so you will need to change the approach here based on your use case. - // let target: &std::path::Path = "../../../../target/".as_ref(); - let library_path = - compute_library_path::(target_dir.as_path()) + let library_dir = compute_library_dir(target_dir.as_path()); + let lib_path = find_cdylib(&library_dir.join("deps"))?; + + // Load the library using libloading + let lib = unsafe { + libloading::Library::new(&lib_path) .map_err(|e| DataFusionError::External(Box::new(e)))? - .join("deps"); - - // Load the module - let module = ForeignLibraryModuleRef::load_from_directory(&library_path) - .map_err(|e| DataFusionError::External(Box::new(e)))?; - - assert_eq!( - module - .version() - .expect("Unable to call version on FFI module")(), - expected_version - ); + }; + + let get_module: libloading::Symbol ForeignLibraryModule> = unsafe { + lib.get(b"datafusion_ffi_get_module") + .map_err(|e| DataFusionError::External(Box::new(e)))? + }; + + let module = get_module(); + + assert_eq!((module.version)(), expected_version); + + // Leak the library to keep it loaded for the duration of the test + std::mem::forget(lib); Ok(module) } diff --git a/datafusion/ffi/src/udaf/accumulator.rs b/datafusion/ffi/src/udaf/accumulator.rs index 6d2b86a3f2e62..a962569e48ecc 100644 --- a/datafusion/ffi/src/udaf/accumulator.rs +++ b/datafusion/ffi/src/udaf/accumulator.rs @@ -19,14 +19,14 @@ use std::ffi::c_void; use std::ops::Deref; use std::ptr::null_mut; -use abi_stable::StableAbi; -use abi_stable::std_types::{RResult, RVec}; +use crate::ffi_option::FfiResult; use arrow::array::ArrayRef; use arrow::error::ArrowError; use datafusion_common::error::{DataFusionError, Result}; use datafusion_common::scalar::ScalarValue; use datafusion_expr::Accumulator; use prost::Message; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::WrappedArray; use crate::util::FFIResult; @@ -36,28 +36,28 @@ use crate::{df_result, rresult, rresult_return}; /// For an explanation of each field, see the corresponding function /// defined in [`Accumulator`]. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_Accumulator { pub update_batch: unsafe extern "C" fn( accumulator: &mut Self, - values: RVec, + values: SVec, ) -> FFIResult<()>, // Evaluate and return a ScalarValues as protobuf bytes - pub evaluate: unsafe extern "C" fn(accumulator: &mut Self) -> FFIResult>, + pub evaluate: unsafe extern "C" fn(accumulator: &mut Self) -> FFIResult>, pub size: unsafe extern "C" fn(accumulator: &Self) -> usize, - pub state: unsafe extern "C" fn(accumulator: &mut Self) -> FFIResult>>, + pub state: unsafe extern "C" fn(accumulator: &mut Self) -> FFIResult>>, pub merge_batch: unsafe extern "C" fn( accumulator: &mut Self, - states: RVec, + states: SVec, ) -> FFIResult<()>, pub retract_batch: unsafe extern "C" fn( accumulator: &mut Self, - values: RVec, + values: SVec, ) -> FFIResult<()>, pub supports_retract_batch: bool, @@ -102,7 +102,7 @@ impl FFI_Accumulator { unsafe extern "C" fn update_batch_fn_wrapper( accumulator: &mut FFI_Accumulator, - values: RVec, + values: SVec, ) -> FFIResult<()> { unsafe { let accumulator = accumulator.inner_mut(); @@ -119,7 +119,7 @@ unsafe extern "C" fn update_batch_fn_wrapper( unsafe extern "C" fn evaluate_fn_wrapper( accumulator: &mut FFI_Accumulator, -) -> FFIResult> { +) -> FFIResult> { unsafe { let accumulator = accumulator.inner_mut(); @@ -127,7 +127,7 @@ unsafe extern "C" fn evaluate_fn_wrapper( let proto_result: datafusion_proto::protobuf::ScalarValue = rresult_return!((&scalar_result).try_into()); - RResult::ROk(proto_result.encode_to_vec().into()) + FfiResult::Ok(proto_result.encode_to_vec().into_iter().collect()) } } @@ -137,7 +137,7 @@ unsafe extern "C" fn size_fn_wrapper(accumulator: &FFI_Accumulator) -> usize { unsafe extern "C" fn state_fn_wrapper( accumulator: &mut FFI_Accumulator, -) -> FFIResult>> { +) -> FFIResult>> { unsafe { let accumulator = accumulator.inner_mut(); @@ -147,10 +147,10 @@ unsafe extern "C" fn state_fn_wrapper( .map(|state_val| { datafusion_proto::protobuf::ScalarValue::try_from(&state_val) .map_err(DataFusionError::from) - .map(|v| RVec::from(v.encode_to_vec())) + .map(|v| v.encode_to_vec().into_iter().collect::>()) }) .collect::>>() - .map(|state_vec| state_vec.into()); + .map(|state_vec| state_vec.into_iter().collect()); rresult!(state) } @@ -158,7 +158,7 @@ unsafe extern "C" fn state_fn_wrapper( unsafe extern "C" fn merge_batch_fn_wrapper( accumulator: &mut FFI_Accumulator, - states: RVec, + states: SVec, ) -> FFIResult<()> { unsafe { let accumulator = accumulator.inner_mut(); @@ -176,7 +176,7 @@ unsafe extern "C" fn merge_batch_fn_wrapper( unsafe extern "C" fn retract_batch_fn_wrapper( accumulator: &mut FFI_Accumulator, - values: RVec, + values: SVec, ) -> FFIResult<()> { unsafe { let accumulator = accumulator.inner_mut(); @@ -265,7 +265,7 @@ impl Accumulator for ForeignAccumulator { .collect::, ArrowError>>()?; df_result!((self.accumulator.update_batch)( &mut self.accumulator, - values.into() + values.into_iter().collect() )) } } @@ -276,7 +276,7 @@ impl Accumulator for ForeignAccumulator { df_result!((self.accumulator.evaluate)(&mut self.accumulator))?; let proto_scalar = - datafusion_proto::protobuf::ScalarValue::decode(scalar_bytes.as_ref()) + datafusion_proto::protobuf::ScalarValue::decode(scalar_bytes.as_slice()) .map_err(|e| DataFusionError::External(Box::new(e)))?; ScalarValue::try_from(&proto_scalar).map_err(DataFusionError::from) @@ -295,12 +295,13 @@ impl Accumulator for ForeignAccumulator { state_protos .into_iter() .map(|proto_bytes| { - datafusion_proto::protobuf::ScalarValue::decode(proto_bytes.as_ref()) - .map_err(|e| DataFusionError::External(Box::new(e))) - .and_then(|proto_value| { - ScalarValue::try_from(&proto_value) - .map_err(DataFusionError::from) - }) + datafusion_proto::protobuf::ScalarValue::decode( + proto_bytes.as_slice(), + ) + .map_err(|e| DataFusionError::External(Box::new(e))) + .and_then(|proto_value| { + ScalarValue::try_from(&proto_value).map_err(DataFusionError::from) + }) }) .collect::>>() } @@ -314,7 +315,7 @@ impl Accumulator for ForeignAccumulator { .collect::, ArrowError>>()?; df_result!((self.accumulator.merge_batch)( &mut self.accumulator, - states.into() + states.into_iter().collect() )) } } @@ -327,7 +328,7 @@ impl Accumulator for ForeignAccumulator { .collect::, ArrowError>>()?; df_result!((self.accumulator.retract_batch)( &mut self.accumulator, - values.into() + values.into_iter().collect() )) } } diff --git a/datafusion/ffi/src/udaf/accumulator_args.rs b/datafusion/ffi/src/udaf/accumulator_args.rs index a3359231073c4..bd7115babdd12 100644 --- a/datafusion/ffi/src/udaf/accumulator_args.rs +++ b/datafusion/ffi/src/udaf/accumulator_args.rs @@ -17,14 +17,14 @@ use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{RString, RVec}; use arrow::datatypes::Schema; use arrow::ffi::FFI_ArrowSchema; use arrow_schema::FieldRef; use datafusion_common::error::DataFusionError; use datafusion_expr::function::AccumulatorArgs; use datafusion_physical_expr::{PhysicalExpr, PhysicalSortExpr}; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::WrappedSchema; use crate::physical_expr::FFI_PhysicalExpr; @@ -35,17 +35,17 @@ use crate::util::{rvec_wrapped_to_vec_fieldref, vec_fieldref_to_rvec_wrapped}; /// For an explanation of each field, see the corresponding field /// defined in [`AccumulatorArgs`]. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_AccumulatorArgs { return_field: WrappedSchema, schema: WrappedSchema, ignore_nulls: bool, - order_bys: RVec, + order_bys: SVec, is_reversed: bool, - name: RString, + name: SString, is_distinct: bool, - exprs: RVec, - expr_fields: RVec, + exprs: SVec, + expr_fields: SVec, } impl TryFrom> for FFI_AccumulatorArgs { @@ -55,7 +55,7 @@ impl TryFrom> for FFI_AccumulatorArgs { WrappedSchema(FFI_ArrowSchema::try_from(args.return_field.as_ref())?); let schema = WrappedSchema(FFI_ArrowSchema::try_from(args.schema)?); - let order_bys: RVec<_> = args + let order_bys: SVec<_> = args .order_bys .iter() .map(FFI_PhysicalSortExpr::from) @@ -76,7 +76,7 @@ impl TryFrom> for FFI_AccumulatorArgs { ignore_nulls: args.ignore_nulls, order_bys, is_reversed: args.is_reversed, - name: args.name.into(), + name: SString::from(args.name), is_distinct: args.is_distinct, exprs, expr_fields, diff --git a/datafusion/ffi/src/udaf/groups_accumulator.rs b/datafusion/ffi/src/udaf/groups_accumulator.rs index fc4ce4b8ba9d0..5494fa2ea323c 100644 --- a/datafusion/ffi/src/udaf/groups_accumulator.rs +++ b/datafusion/ffi/src/udaf/groups_accumulator.rs @@ -20,13 +20,13 @@ use std::ops::Deref; use std::ptr::null_mut; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RVec}; +use crate::ffi_option::FfiOption; use arrow::array::{Array, ArrayRef, BooleanArray}; use arrow::error::ArrowError; use arrow::ffi::to_ffi; use datafusion_common::error::{DataFusionError, Result}; use datafusion_expr::{EmitTo, GroupsAccumulator}; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::{WrappedArray, WrappedSchema}; use crate::util::FFIResult; @@ -36,13 +36,13 @@ use crate::{df_result, rresult, rresult_return}; /// For an explanation of each field, see the corresponding function /// defined in [`GroupsAccumulator`]. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_GroupsAccumulator { pub update_batch: unsafe extern "C" fn( accumulator: &mut Self, - values: RVec, - group_indices: RVec, - opt_filter: ROption, + values: SVec, + group_indices: SVec, + opt_filter: FfiOption, total_num_groups: usize, ) -> FFIResult<()>, @@ -57,21 +57,21 @@ pub struct FFI_GroupsAccumulator { pub state: unsafe extern "C" fn( accumulator: &mut Self, emit_to: FFI_EmitTo, - ) -> FFIResult>, + ) -> FFIResult>, pub merge_batch: unsafe extern "C" fn( accumulator: &mut Self, - values: RVec, - group_indices: RVec, - opt_filter: ROption, + values: SVec, + group_indices: SVec, + opt_filter: FfiOption, total_num_groups: usize, ) -> FFIResult<()>, pub convert_to_state: unsafe extern "C" fn( accumulator: &Self, - values: RVec, - opt_filter: ROption, - ) -> FFIResult>, + values: SVec, + opt_filter: FfiOption, + ) -> FFIResult>, pub supports_convert_to_state: bool, @@ -110,7 +110,7 @@ impl FFI_GroupsAccumulator { } } -fn process_values(values: RVec) -> Result>> { +fn process_values(values: SVec) -> Result>> { values .into_iter() .map(|v| v.try_into().map_err(DataFusionError::from)) @@ -118,9 +118,11 @@ fn process_values(values: RVec) -> Result>> { } /// Convert C-typed opt_filter into the internal type. -fn process_opt_filter(opt_filter: ROption) -> Result> { +fn process_opt_filter( + opt_filter: FfiOption, +) -> Result> { + let opt_filter: Option = opt_filter.into(); opt_filter - .into_option() .map(|filter| { ArrayRef::try_from(filter) .map_err(DataFusionError::from) @@ -131,9 +133,9 @@ fn process_opt_filter(opt_filter: ROption) -> Result, - group_indices: RVec, - opt_filter: ROption, + values: SVec, + group_indices: SVec, + opt_filter: FfiOption, total_num_groups: usize, ) -> FFIResult<()> { unsafe { @@ -174,7 +176,7 @@ unsafe extern "C" fn size_fn_wrapper(accumulator: &FFI_GroupsAccumulator) -> usi unsafe extern "C" fn state_fn_wrapper( accumulator: &mut FFI_GroupsAccumulator, emit_to: FFI_EmitTo, -) -> FFIResult> { +) -> FFIResult> { unsafe { let accumulator = accumulator.inner_mut(); @@ -183,16 +185,16 @@ unsafe extern "C" fn state_fn_wrapper( state .into_iter() .map(|arr| WrappedArray::try_from(&arr).map_err(DataFusionError::from)) - .collect::>>() + .collect::>>() ) } } unsafe extern "C" fn merge_batch_fn_wrapper( accumulator: &mut FFI_GroupsAccumulator, - values: RVec, - group_indices: RVec, - opt_filter: ROption, + values: SVec, + group_indices: SVec, + opt_filter: FfiOption, total_num_groups: usize, ) -> FFIResult<()> { unsafe { @@ -212,9 +214,9 @@ unsafe extern "C" fn merge_batch_fn_wrapper( unsafe extern "C" fn convert_to_state_fn_wrapper( accumulator: &FFI_GroupsAccumulator, - values: RVec, - opt_filter: ROption, -) -> FFIResult> { + values: SVec, + opt_filter: FfiOption, +) -> FFIResult> { unsafe { let accumulator = accumulator.inner(); let values = rresult_return!(process_values(values)); @@ -226,7 +228,7 @@ unsafe extern "C" fn convert_to_state_fn_wrapper( state .iter() .map(|arr| WrappedArray::try_from(arr).map_err(DataFusionError::from)) - .collect::>>() + .collect::>>() ) } } @@ -321,12 +323,12 @@ impl GroupsAccumulator for ForeignGroupsAccumulator { .map(|(array, schema)| WrappedArray { array, schema: WrappedSchema(schema), - }) - .into(); + }); + let opt_filter = FfiOption::from(opt_filter); df_result!((self.accumulator.update_batch)( &mut self.accumulator, - values.into(), + values.into_iter().collect(), group_indices, opt_filter, total_num_groups @@ -384,12 +386,12 @@ impl GroupsAccumulator for ForeignGroupsAccumulator { .map(|(array, schema)| WrappedArray { array, schema: WrappedSchema(schema), - }) - .into(); + }); + let opt_filter = FfiOption::from(opt_filter); df_result!((self.accumulator.merge_batch)( &mut self.accumulator, - values.into(), + values.into_iter().collect(), group_indices, opt_filter, total_num_groups @@ -406,7 +408,7 @@ impl GroupsAccumulator for ForeignGroupsAccumulator { let values = values .iter() .map(WrappedArray::try_from) - .collect::, ArrowError>>()?; + .collect::, ArrowError>>()?; let opt_filter = opt_filter .map(|bool_array| to_ffi(&bool_array.to_data())) @@ -414,8 +416,8 @@ impl GroupsAccumulator for ForeignGroupsAccumulator { .map(|(array, schema)| WrappedArray { array, schema: WrappedSchema(schema), - }) - .into(); + }); + let opt_filter = FfiOption::from(opt_filter); let returned_array = df_result!((self.accumulator.convert_to_state)( &self.accumulator, @@ -436,7 +438,7 @@ impl GroupsAccumulator for ForeignGroupsAccumulator { } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub enum FFI_EmitTo { All, First(usize), diff --git a/datafusion/ffi/src/udaf/mod.rs b/datafusion/ffi/src/udaf/mod.rs index 22cbe8cff0fe6..76f6568467ef6 100644 --- a/datafusion/ffi/src/udaf/mod.rs +++ b/datafusion/ffi/src/udaf/mod.rs @@ -19,8 +19,8 @@ use std::ffi::c_void; use std::hash::{Hash, Hasher}; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RResult, RStr, RString, RVec}; +use crate::ffi_option::FfiOption; +use crate::ffi_option::FfiResult; use accumulator::FFI_Accumulator; use accumulator_args::{FFI_AccumulatorArgs, ForeignAccumulatorArgs}; use arrow::datatypes::{DataType, Field}; @@ -39,6 +39,9 @@ use datafusion_functions_aggregate_common::order::AggregateOrderSensitivity; use datafusion_proto_common::from_proto::parse_proto_fields_to_fields; use groups_accumulator::FFI_GroupsAccumulator; use prost::{DecodeError, Message}; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; +use stabby::str::Str as SStr; use crate::arrow_wrappers::WrappedSchema; use crate::util::{ @@ -54,13 +57,13 @@ mod groups_accumulator; /// A stable struct for sharing a [`AggregateUDF`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_AggregateUDF { /// FFI equivalent to the `name` of a [`AggregateUDF`] - pub name: RString, + pub name: SString, /// FFI equivalent to the `aliases` of a [`AggregateUDF`] - pub aliases: RVec, + pub aliases: SVec, /// FFI equivalent to the `volatility` of a [`AggregateUDF`] pub volatility: FFI_Volatility, @@ -69,7 +72,7 @@ pub struct FFI_AggregateUDF { /// argument fields. pub return_field: unsafe extern "C" fn( udaf: &Self, - arg_fields: RVec, + arg_fields: SVec, ) -> FFIResult, /// FFI equivalent to the `is_nullable` of a [`AggregateUDF`] @@ -95,12 +98,12 @@ pub struct FFI_AggregateUDF { /// FFI equivalent to [`AggregateUDF::state_fields`] pub state_fields: unsafe extern "C" fn( udaf: &FFI_AggregateUDF, - name: &RStr, - input_fields: RVec, + name: &SStr, + input_fields: SVec, return_field: WrappedSchema, - ordering_fields: RVec>, + ordering_fields: SVec>, is_distinct: bool, - ) -> FFIResult>>, + ) -> FFIResult>>, /// FFI equivalent to [`AggregateUDF::create_groups_accumulator`] pub create_groups_accumulator: @@ -114,7 +117,7 @@ pub struct FFI_AggregateUDF { unsafe extern "C" fn( udaf: &FFI_AggregateUDF, beneficial_ordering: bool, - ) -> FFIResult>, + ) -> FFIResult>, /// FFI equivalent to [`AggregateUDF::order_sensitivity`] pub order_sensitivity: @@ -126,8 +129,8 @@ pub struct FFI_AggregateUDF { /// appropriate calls on the underlying [`AggregateUDF`] pub coerce_types: unsafe extern "C" fn( udf: &Self, - arg_types: RVec, - ) -> FFIResult>, + arg_types: SVec, + ) -> FFIResult>, /// Used to create a clone on the provider of the udaf. This should /// only need to be called by the receiver of the udaf. @@ -164,7 +167,7 @@ impl FFI_AggregateUDF { unsafe extern "C" fn return_field_fn_wrapper( udaf: &FFI_AggregateUDF, - arg_fields: RVec, + arg_fields: SVec, ) -> FFIResult { unsafe { let udaf = udaf.inner(); @@ -249,7 +252,7 @@ unsafe extern "C" fn groups_accumulator_supported_fn_wrapper( unsafe extern "C" fn with_beneficial_ordering_fn_wrapper( udaf: &FFI_AggregateUDF, beneficial_ordering: bool, -) -> FFIResult> { +) -> FFIResult> { unsafe { let udaf = udaf.inner().as_ref().clone(); @@ -262,18 +265,18 @@ unsafe extern "C" fn with_beneficial_ordering_fn_wrapper( .flatten() .map(|func| FFI_AggregateUDF::from(Arc::new(func))); - RResult::ROk(result.into()) + FfiResult::Ok(FfiOption::from(result)) } } unsafe extern "C" fn state_fields_fn_wrapper( udaf: &FFI_AggregateUDF, - name: &RStr, - input_fields: RVec, + name: &SStr, + input_fields: SVec, return_field: WrappedSchema, - ordering_fields: RVec>, + ordering_fields: SVec>, is_distinct: bool, -) -> FFIResult>> { +) -> FFIResult>> { unsafe { let udaf = udaf.inner(); @@ -284,7 +287,7 @@ unsafe extern "C" fn state_fields_fn_wrapper( ordering_fields .into_iter() .map(|field_bytes| datafusion_proto_common::Field::decode( - field_bytes.as_ref() + field_bytes.as_slice() )) .collect::, DecodeError>>() ); @@ -313,10 +316,10 @@ unsafe extern "C" fn state_fields_fn_wrapper( .collect::>>() ) .into_iter() - .map(|field| field.encode_to_vec().into()) + .map(|field| field.encode_to_vec().into_iter().collect()) .collect(); - RResult::ROk(state_fields) + FfiResult::Ok(state_fields) } } @@ -328,8 +331,8 @@ unsafe extern "C" fn order_sensitivity_fn_wrapper( unsafe extern "C" fn coerce_types_fn_wrapper( udaf: &FFI_AggregateUDF, - arg_types: RVec, -) -> FFIResult> { + arg_types: SVec, +) -> FFIResult> { unsafe { let udaf = udaf.inner(); @@ -371,8 +374,12 @@ impl Clone for FFI_AggregateUDF { impl From> for FFI_AggregateUDF { fn from(udaf: Arc) -> Self { - let name = udaf.name().into(); - let aliases = udaf.aliases().iter().map(|a| a.to_owned().into()).collect(); + let name = SString::from(udaf.name()); + let aliases = udaf + .aliases() + .iter() + .map(|a| SString::from(a.as_str())) + .collect(); let is_nullable = udaf.is_nullable(); let volatility = udaf.signature().volatility.into(); @@ -442,7 +449,7 @@ impl From<&FFI_AggregateUDF> for Arc { } let signature = Signature::user_defined((&udaf.volatility).into()); - let aliases = udaf.aliases.iter().map(|s| s.to_string()).collect(); + let aliases: Vec = udaf.aliases.iter().map(|s| s.to_string()).collect(); Arc::new(ForeignAggregateUDF { udaf: udaf.clone(), @@ -497,7 +504,7 @@ impl AggregateUDFImpl for ForeignAggregateUDF { fn state_fields(&self, args: StateFieldsArgs) -> Result> { unsafe { - let name = RStr::from_str(args.name); + let name = SStr::from(args.name); let input_fields = vec_fieldref_to_rvec_wrapped(args.input_fields)?; let return_field = WrappedSchema(FFI_ArrowSchema::try_from(args.return_field.as_ref())?); @@ -509,7 +516,7 @@ impl AggregateUDFImpl for ForeignAggregateUDF { .map(|v| v.map_err(DataFusionError::from)) .collect::>>()? .into_iter() - .map(|proto_field| proto_field.encode_to_vec().into()) + .map(|proto_field| proto_field.encode_to_vec().into_iter().collect()) .collect(); let fields = df_result!((self.udaf.state_fields)( @@ -523,7 +530,7 @@ impl AggregateUDFImpl for ForeignAggregateUDF { let fields = fields .into_iter() .map(|field_bytes| { - datafusion_proto_common::Field::decode(field_bytes.as_ref()) + datafusion_proto_common::Field::decode(field_bytes.as_slice()) .map_err(|e| ffi_datafusion_err!("{e}")) }) .collect::>>()?; @@ -578,11 +585,11 @@ impl AggregateUDFImpl for ForeignAggregateUDF { beneficial_ordering: bool, ) -> Result>> { unsafe { - let result = df_result!((self.udaf.with_beneficial_ordering)( + let stabby_opt = df_result!((self.udaf.with_beneficial_ordering)( &self.udaf, beneficial_ordering - ))? - .into_option(); + ))?; + let result: Option = stabby_opt.into(); let result = result.map(|func| >::from(&func)); @@ -609,7 +616,7 @@ impl AggregateUDFImpl for ForeignAggregateUDF { } #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub enum FFI_AggregateOrderSensitivity { Insensitive, HardRequirement, diff --git a/datafusion/ffi/src/udf/mod.rs b/datafusion/ffi/src/udf/mod.rs index c6ef0f2a50dfa..f383890400257 100644 --- a/datafusion/ffi/src/udf/mod.rs +++ b/datafusion/ffi/src/udf/mod.rs @@ -19,8 +19,9 @@ use std::ffi::c_void; use std::hash::{Hash, Hasher}; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{RString, RVec}; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; + use arrow::array::Array; use arrow::datatypes::{DataType, Field}; use arrow::error::ArrowError; @@ -50,13 +51,13 @@ pub mod return_type_args; /// A stable struct for sharing a [`ScalarUDF`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_ScalarUDF { /// FFI equivalent to the `name` of a [`ScalarUDF`] - pub name: RString, + pub name: SString, /// FFI equivalent to the `aliases` of a [`ScalarUDF`] - pub aliases: RVec, + pub aliases: SVec, /// FFI equivalent to the `volatility` of a [`ScalarUDF`] pub volatility: FFI_Volatility, @@ -71,8 +72,8 @@ pub struct FFI_ScalarUDF { /// within an AbiStable wrapper. pub invoke_with_args: unsafe extern "C" fn( udf: &Self, - args: RVec, - arg_fields: RVec, + args: SVec, + arg_fields: SVec, num_rows: usize, return_field: WrappedSchema, config_options: FFI_ConfigOptions, @@ -87,8 +88,8 @@ pub struct FFI_ScalarUDF { /// appropriate calls on the underlying [`ScalarUDF`] pub coerce_types: unsafe extern "C" fn( udf: &Self, - arg_types: RVec, - ) -> FFIResult>, + arg_types: SVec, + ) -> FFIResult>, /// Used to create a clone on the provider of the udf. This should /// only need to be called by the receiver of the udf. @@ -139,8 +140,8 @@ unsafe extern "C" fn return_field_from_args_fn_wrapper( unsafe extern "C" fn coerce_types_fn_wrapper( udf: &FFI_ScalarUDF, - arg_types: RVec, -) -> FFIResult> { + arg_types: SVec, +) -> FFIResult> { let arg_types = rresult_return!(rvec_wrapped_to_vec_datatype(&arg_types)); let arg_fields = arg_types @@ -158,8 +159,8 @@ unsafe extern "C" fn coerce_types_fn_wrapper( unsafe extern "C" fn invoke_with_args_fn_wrapper( udf: &FFI_ScalarUDF, - args: RVec, - arg_fields: RVec, + args: SVec, + arg_fields: SVec, number_rows: usize, return_field: WrappedSchema, config_options: FFI_ConfigOptions, @@ -230,8 +231,12 @@ impl Clone for FFI_ScalarUDF { impl From> for FFI_ScalarUDF { fn from(udf: Arc) -> Self { - let name = udf.name().into(); - let aliases = udf.aliases().iter().map(|a| a.to_owned().into()).collect(); + let name = SString::from(udf.name()); + let aliases = udf + .aliases() + .iter() + .map(|a| SString::from(a.as_str())) + .collect(); let volatility = udf.signature().volatility.into(); let short_circuits = udf.short_circuits(); @@ -312,7 +317,7 @@ impl From<&FFI_ScalarUDF> for Arc { if (udf.library_marker_id)() == crate::get_library_marker_id() { Arc::clone(udf.inner().inner()) } else { - let name = udf.name.to_owned().into(); + let name = udf.name.to_string(); let signature = Signature::user_defined((&udf.volatility).into()); let aliases = udf.aliases.iter().map(|s| s.to_string()).collect(); @@ -379,7 +384,8 @@ impl ScalarUDFImpl for ForeignScalarUDF { }) }) .collect::, ArrowError>>()? - .into(); + .into_iter() + .collect(); let arg_fields_wrapped = arg_fields .iter() @@ -389,7 +395,7 @@ impl ScalarUDFImpl for ForeignScalarUDF { let arg_fields = arg_fields_wrapped .into_iter() .map(WrappedSchema) - .collect::>(); + .collect::>(); let return_field = return_field.as_ref().clone(); let return_field = WrappedSchema(FFI_ArrowSchema::try_from(return_field)?); diff --git a/datafusion/ffi/src/udf/return_type_args.rs b/datafusion/ffi/src/udf/return_type_args.rs index 8fb015b7ed922..197bca602e1ca 100644 --- a/datafusion/ffi/src/udf/return_type_args.rs +++ b/datafusion/ffi/src/udf/return_type_args.rs @@ -15,23 +15,23 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RVec}; +use crate::ffi_option::FfiOption; use arrow_schema::FieldRef; use datafusion_common::scalar::ScalarValue; use datafusion_common::{DataFusionError, ffi_datafusion_err}; use datafusion_expr::ReturnFieldArgs; use prost::Message; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::WrappedSchema; use crate::util::{rvec_wrapped_to_vec_fieldref, vec_fieldref_to_rvec_wrapped}; /// A stable struct for sharing a [`ReturnFieldArgs`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_ReturnFieldArgs { - arg_fields: RVec, - scalar_arguments: RVec>>, + arg_fields: SVec, + scalar_arguments: SVec>>, } impl TryFrom> for FFI_ReturnFieldArgs { @@ -47,13 +47,15 @@ impl TryFrom> for FFI_ReturnFieldArgs { .map(|arg| { let proto_value: datafusion_proto::protobuf::ScalarValue = arg.try_into()?; - let proto_bytes: RVec = proto_value.encode_to_vec().into(); + let proto_bytes: SVec = + proto_value.encode_to_vec().into_iter().collect(); Ok(proto_bytes) }) .transpose() }) .collect(); - let scalar_arguments = scalar_arguments?.into_iter().map(ROption::from).collect(); + let scalar_arguments = + scalar_arguments?.into_iter().map(FfiOption::from).collect(); Ok(Self { arg_fields, @@ -86,15 +88,15 @@ impl TryFrom<&FFI_ReturnFieldArgs> for ForeignReturnFieldArgsOwned { .map(|maybe_arg| { let maybe_arg = maybe_arg.as_ref().map(|arg| { let proto_value = - datafusion_proto::protobuf::ScalarValue::decode(arg.as_ref()) + datafusion_proto::protobuf::ScalarValue::decode(arg.as_slice()) .map_err(|err| ffi_datafusion_err!("{}", err))?; let scalar_value: ScalarValue = (&proto_value).try_into()?; Ok(scalar_value) }); - Option::from(maybe_arg).transpose() + maybe_arg.transpose() }) .collect(); - let scalar_arguments = scalar_arguments?.into_iter().collect(); + let scalar_arguments = scalar_arguments?; Ok(Self { arg_fields, diff --git a/datafusion/ffi/src/udtf.rs b/datafusion/ffi/src/udtf.rs index 6024ec755de58..a96745865d3bc 100644 --- a/datafusion/ffi/src/udtf.rs +++ b/datafusion/ffi/src/udtf.rs @@ -18,8 +18,7 @@ use std::ffi::c_void; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{RResult, RVec}; +use crate::ffi_option::FfiResult; use datafusion_catalog::{TableFunctionImpl, TableProvider}; use datafusion_common::error::Result; use datafusion_execution::TaskContext; @@ -31,6 +30,7 @@ use datafusion_proto::logical_plan::{ }; use datafusion_proto::protobuf::LogicalExprList; use prost::Message; +use stabby::alloc::vec::Vec as SVec; use tokio::runtime::Handle; use crate::execution::FFI_TaskContextProvider; @@ -41,12 +41,12 @@ use crate::{df_result, rresult_return}; /// A stable struct for sharing a [`TableFunctionImpl`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_TableFunction { /// Equivalent to the `call` function of the TableFunctionImpl. /// The arguments are Expr passed as protobuf encoded bytes. pub call: - unsafe extern "C" fn(udtf: &Self, args: RVec) -> FFIResult, + unsafe extern "C" fn(udtf: &Self, args: SVec) -> FFIResult, pub logical_codec: FFI_LogicalExtensionCodec, @@ -89,7 +89,7 @@ impl FFI_TableFunction { unsafe extern "C" fn call_fn_wrapper( udtf: &FFI_TableFunction, - args: RVec, + args: SVec, ) -> FFIResult { let runtime = udtf.runtime(); let udtf_inner = udtf.inner(); @@ -98,7 +98,7 @@ unsafe extern "C" fn call_fn_wrapper( rresult_return!((&udtf.logical_codec.task_ctx_provider).try_into()); let codec: Arc = (&udtf.logical_codec).into(); - let proto_filters = rresult_return!(LogicalExprList::decode(args.as_ref())); + let proto_filters = rresult_return!(LogicalExprList::decode(args.as_slice())); let args = rresult_return!(parse_exprs( proto_filters.expr.iter(), @@ -107,7 +107,7 @@ unsafe extern "C" fn call_fn_wrapper( )); let table_provider = rresult_return!(udtf_inner.call(&args)); - RResult::ROk(FFI_TableProvider::new_with_ffi_codec( + FfiResult::Ok(FFI_TableProvider::new_with_ffi_codec( table_provider, false, runtime, @@ -213,7 +213,8 @@ impl TableFunctionImpl for ForeignTableFunction { let expr_list = LogicalExprList { expr: serialize_exprs(args, codec.as_ref())?, }; - let filters_serialized = expr_list.encode_to_vec().into(); + let filters_serialized: SVec = + expr_list.encode_to_vec().into_iter().collect(); let table_provider = unsafe { (self.0.call)(&self.0, filters_serialized) }; diff --git a/datafusion/ffi/src/udwf/mod.rs b/datafusion/ffi/src/udwf/mod.rs index dbac00fd43020..b5a4dd54fead8 100644 --- a/datafusion/ffi/src/udwf/mod.rs +++ b/datafusion/ffi/src/udwf/mod.rs @@ -19,8 +19,8 @@ use std::ffi::c_void; use std::hash::{Hash, Hasher}; use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::{ROption, RResult, RString, RVec}; +use crate::ffi_option::FfiOption; +use crate::ffi_option::FfiResult; use arrow::compute::SortOptions; use arrow::datatypes::{DataType, Schema, SchemaRef}; use arrow_schema::{Field, FieldRef}; @@ -35,6 +35,8 @@ use partition_evaluator::FFI_PartitionEvaluator; use partition_evaluator_args::{ FFI_PartitionEvaluatorArgs, ForeignPartitionEvaluatorArgs, }; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; mod partition_evaluator; mod partition_evaluator_args; @@ -50,13 +52,13 @@ use crate::{df_result, rresult, rresult_return}; /// A stable struct for sharing a [`WindowUDF`] across FFI boundaries. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_WindowUDF { /// FFI equivalent to the `name` of a [`WindowUDF`] - pub name: RString, + pub name: SString, /// FFI equivalent to the `aliases` of a [`WindowUDF`] - pub aliases: RVec, + pub aliases: SVec, /// FFI equivalent to the `volatility` of a [`WindowUDF`] pub volatility: FFI_Volatility, @@ -69,8 +71,8 @@ pub struct FFI_WindowUDF { pub field: unsafe extern "C" fn( udwf: &Self, - input_types: RVec, - display_name: RString, + input_types: SVec, + display_name: SString, ) -> FFIResult, /// Performs type coercion. To simply this interface, all UDFs are treated as having @@ -79,10 +81,10 @@ pub struct FFI_WindowUDF { /// appropriate calls on the underlying [`WindowUDF`] pub coerce_types: unsafe extern "C" fn( udf: &Self, - arg_types: RVec, - ) -> FFIResult>, + arg_types: SVec, + ) -> FFIResult>, - pub sort_options: ROption, + pub sort_options: FfiOption, /// Used to create a clone on the provider of the udf. This should /// only need to be called by the receiver of the udf. @@ -129,35 +131,34 @@ unsafe extern "C" fn partition_evaluator_fn_wrapper( let evaluator = rresult_return!(inner.partition_evaluator_factory((&args).into())); - RResult::ROk(evaluator.into()) + FfiResult::Ok(evaluator.into()) } } unsafe extern "C" fn field_fn_wrapper( udwf: &FFI_WindowUDF, - input_fields: RVec, - display_name: RString, + input_fields: SVec, + display_name: SString, ) -> FFIResult { unsafe { let inner = udwf.inner(); let input_fields = rresult_return!(rvec_wrapped_to_vec_fieldref(&input_fields)); - let field = rresult_return!(inner.field(WindowUDFFieldArgs::new( - &input_fields, - display_name.as_str() - ))); + let field = rresult_return!( + inner.field(WindowUDFFieldArgs::new(&input_fields, &display_name)) + ); let schema = Arc::new(Schema::new(vec![field])); - RResult::ROk(WrappedSchema::from(schema)) + FfiResult::Ok(WrappedSchema::from(schema)) } } unsafe extern "C" fn coerce_types_fn_wrapper( udwf: &FFI_WindowUDF, - arg_types: RVec, -) -> FFIResult> { + arg_types: SVec, +) -> FFIResult> { unsafe { let inner = udwf.inner(); @@ -203,7 +204,10 @@ unsafe extern "C" fn clone_fn_wrapper(udwf: &FFI_WindowUDF) -> FFI_WindowUDF { aliases: udwf.aliases.clone(), volatility: udwf.volatility.clone(), partition_evaluator: partition_evaluator_fn_wrapper, - sort_options: udwf.sort_options.clone(), + sort_options: { + let opts: Option<&FFI_SortOptions> = udwf.sort_options.as_ref(); + FfiOption::from(opts.cloned()) + }, coerce_types: coerce_types_fn_wrapper, field: field_fn_wrapper, clone: clone_fn_wrapper, @@ -222,10 +226,15 @@ impl Clone for FFI_WindowUDF { impl From> for FFI_WindowUDF { fn from(udf: Arc) -> Self { - let name = udf.name().into(); - let aliases = udf.aliases().iter().map(|a| a.to_owned().into()).collect(); + let name = SString::from(udf.name()); + let aliases = udf + .aliases() + .iter() + .map(|a| SString::from(a.as_str())) + .collect(); let volatility = udf.signature().volatility.into(); - let sort_options = udf.sort_options().map(|v| (&v).into()).into(); + let sort_options = + FfiOption::from(udf.sort_options().map(|v| FFI_SortOptions::from(&v))); let private_data = Box::new(WindowUDFPrivateData { udf }); @@ -286,7 +295,7 @@ impl From<&FFI_WindowUDF> for Arc { if (udf.library_marker_id)() == crate::get_library_marker_id() { Arc::clone(unsafe { udf.inner().inner() }) } else { - let name = udf.name.to_owned().into(); + let name = udf.name.to_string(); let signature = Signature::user_defined((&udf.volatility).into()); let aliases = udf.aliases.iter().map(|s| s.to_string()).collect(); @@ -344,7 +353,7 @@ impl WindowUDFImpl for ForeignWindowUDF { let schema = df_result!((self.udf.field)( &self.udf, input_types, - field_args.name().into() + SString::from(field_args.name()) ))?; let schema: SchemaRef = schema.into(); @@ -358,7 +367,7 @@ impl WindowUDFImpl for ForeignWindowUDF { } fn sort_options(&self) -> Option { - let options: Option<&FFI_SortOptions> = self.udf.sort_options.as_ref().into(); + let options: Option<&FFI_SortOptions> = self.udf.sort_options.as_ref(); options.map(|s| s.into()) } @@ -368,7 +377,7 @@ impl WindowUDFImpl for ForeignWindowUDF { } #[repr(C)] -#[derive(Debug, StableAbi, Clone)] +#[derive(Debug, Clone)] pub struct FFI_SortOptions { pub descending: bool, pub nulls_first: bool, diff --git a/datafusion/ffi/src/udwf/partition_evaluator.rs b/datafusion/ffi/src/udwf/partition_evaluator.rs index 8df02511aa4b3..16bc87d3063f6 100644 --- a/datafusion/ffi/src/udwf/partition_evaluator.rs +++ b/datafusion/ffi/src/udwf/partition_evaluator.rs @@ -18,8 +18,7 @@ use std::ffi::c_void; use std::ops::Range; -use abi_stable::StableAbi; -use abi_stable::std_types::{RResult, RVec}; +use crate::ffi_option::FfiResult; use arrow::array::ArrayRef; use arrow::error::ArrowError; use datafusion_common::scalar::ScalarValue; @@ -27,6 +26,7 @@ use datafusion_common::{DataFusionError, Result}; use datafusion_expr::PartitionEvaluator; use datafusion_expr::window_state::WindowAggState; use prost::Message; +use stabby::alloc::vec::Vec as SVec; use super::range::FFI_Range; use crate::arrow_wrappers::WrappedArray; @@ -37,24 +37,24 @@ use crate::{df_result, rresult, rresult_return}; /// For an explanation of each field, see the corresponding function /// defined in [`PartitionEvaluator`]. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_PartitionEvaluator { pub evaluate_all: unsafe extern "C" fn( evaluator: &mut Self, - values: RVec, + values: SVec, num_rows: usize, ) -> FFIResult, pub evaluate: unsafe extern "C" fn( evaluator: &mut Self, - values: RVec, + values: SVec, range: FFI_Range, - ) -> FFIResult>, + ) -> FFIResult>, pub evaluate_all_with_rank: unsafe extern "C" fn( evaluator: &Self, num_rows: usize, - ranks_in_partition: RVec, + ranks_in_partition: SVec, ) -> FFIResult, pub get_range: unsafe extern "C" fn( @@ -107,7 +107,7 @@ impl FFI_PartitionEvaluator { unsafe extern "C" fn evaluate_all_fn_wrapper( evaluator: &mut FFI_PartitionEvaluator, - values: RVec, + values: SVec, num_rows: usize, ) -> FFIResult { unsafe { @@ -132,9 +132,9 @@ unsafe extern "C" fn evaluate_all_fn_wrapper( unsafe extern "C" fn evaluate_fn_wrapper( evaluator: &mut FFI_PartitionEvaluator, - values: RVec, + values: SVec, range: FFI_Range, -) -> FFIResult> { +) -> FFIResult> { unsafe { let inner = evaluator.inner_mut(); @@ -151,14 +151,14 @@ unsafe extern "C" fn evaluate_fn_wrapper( let proto_result: datafusion_proto::protobuf::ScalarValue = rresult_return!((&scalar_result).try_into()); - RResult::ROk(proto_result.encode_to_vec().into()) + FfiResult::Ok(proto_result.encode_to_vec().into_iter().collect()) } } unsafe extern "C" fn evaluate_all_with_rank_fn_wrapper( evaluator: &FFI_PartitionEvaluator, num_rows: usize, - ranks_in_partition: RVec, + ranks_in_partition: SVec, ) -> FFIResult { unsafe { let inner = evaluator.inner(); @@ -284,7 +284,7 @@ impl PartitionEvaluator for ForeignPartitionEvaluator { let values = values .iter() .map(WrappedArray::try_from) - .collect::, ArrowError>>()?; + .collect::, ArrowError>>()?; (self.evaluator.evaluate_all)(&mut self.evaluator, values, num_rows) }; @@ -302,7 +302,7 @@ impl PartitionEvaluator for ForeignPartitionEvaluator { let values = values .iter() .map(WrappedArray::try_from) - .collect::, ArrowError>>()?; + .collect::, ArrowError>>()?; let scalar_bytes = df_result!((self.evaluator.evaluate)( &mut self.evaluator, @@ -311,7 +311,7 @@ impl PartitionEvaluator for ForeignPartitionEvaluator { ))?; let proto_scalar = - datafusion_proto::protobuf::ScalarValue::decode(scalar_bytes.as_ref()) + datafusion_proto::protobuf::ScalarValue::decode(scalar_bytes.as_slice()) .map_err(|e| DataFusionError::External(Box::new(e)))?; ScalarValue::try_from(&proto_scalar).map_err(DataFusionError::from) diff --git a/datafusion/ffi/src/udwf/partition_evaluator_args.rs b/datafusion/ffi/src/udwf/partition_evaluator_args.rs index ffad1f41ee42d..8e9b165037d2a 100644 --- a/datafusion/ffi/src/udwf/partition_evaluator_args.rs +++ b/datafusion/ffi/src/udwf/partition_evaluator_args.rs @@ -17,14 +17,13 @@ use std::sync::Arc; -use abi_stable::StableAbi; -use abi_stable::std_types::RVec; use arrow::error::ArrowError; use arrow::ffi::FFI_ArrowSchema; use arrow_schema::FieldRef; use datafusion_common::{DataFusionError, Result}; use datafusion_expr::function::PartitionEvaluatorArgs; use datafusion_physical_plan::PhysicalExpr; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::WrappedSchema; use crate::physical_expr::FFI_PhysicalExpr; @@ -34,10 +33,10 @@ use crate::util::rvec_wrapped_to_vec_fieldref; /// For an explanation of each field, see the corresponding function /// defined in [`PartitionEvaluatorArgs`]. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_PartitionEvaluatorArgs { - input_exprs: RVec, - input_fields: RVec, + input_exprs: SVec, + input_fields: SVec, is_reversed: bool, ignore_nulls: bool, } @@ -58,7 +57,8 @@ impl TryFrom> for FFI_PartitionEvaluatorArgs { .iter() .map(|input_type| FFI_ArrowSchema::try_from(input_type).map(WrappedSchema)) .collect::, ArrowError>>()? - .into(); + .into_iter() + .collect(); Ok(Self { input_exprs, diff --git a/datafusion/ffi/src/udwf/range.rs b/datafusion/ffi/src/udwf/range.rs index 19a908c5e2454..558fd058a67cf 100644 --- a/datafusion/ffi/src/udwf/range.rs +++ b/datafusion/ffi/src/udwf/range.rs @@ -17,13 +17,11 @@ use std::ops::Range; -use abi_stable::StableAbi; - /// A stable struct for sharing [`Range`] across FFI boundaries. /// For an explanation of each field, see the corresponding function /// defined in [`Range`]. #[repr(C)] -#[derive(Debug, StableAbi)] +#[derive(Debug)] pub struct FFI_Range { pub start: usize, pub end: usize, diff --git a/datafusion/ffi/src/util.rs b/datafusion/ffi/src/util.rs index db6eb0552d2aa..3036d5b33707a 100644 --- a/datafusion/ffi/src/util.rs +++ b/datafusion/ffi/src/util.rs @@ -17,10 +17,12 @@ use std::sync::Arc; -use abi_stable::std_types::{RResult, RString, RVec}; +use crate::ffi_option::FfiResult; use arrow::datatypes::{DataType, Field}; use arrow::ffi::FFI_ArrowSchema; use arrow_schema::FieldRef; +use stabby::alloc::string::String as SString; +use stabby::alloc::vec::Vec as SVec; use crate::arrow_wrappers::WrappedSchema; @@ -29,37 +31,37 @@ use crate::arrow_wrappers::WrappedSchema; /// a FFI safe variant of it, we convert errors to strings when passing results /// back. These are converted back and forth using the `df_result`, `rresult`, /// and `rresult_return` macros. -pub type FFIResult = RResult; +pub type FFIResult = FfiResult; -/// This macro is a helpful conversion utility to convert from an abi_stable::RResult to a +/// This macro is a helpful conversion utility to convert from an FFIResult to a /// DataFusion result. #[macro_export] macro_rules! df_result { ( $x:expr ) => { - match $x { - abi_stable::std_types::RResult::ROk(v) => Ok(v), - abi_stable::std_types::RResult::RErr(err) => { + match Into::<::std::result::Result<_, _>>::into($x) { + Ok(v) => Ok(v), + Err(err) => { datafusion_common::ffi_err!("{err}") } } }; } -/// This macro is a helpful conversion utility to convert from a DataFusion Result to an abi_stable::RResult +/// This macro is a helpful conversion utility to convert from a DataFusion Result to an FFIResult. #[macro_export] macro_rules! rresult { ( $x:expr ) => { match $x { - Ok(v) => abi_stable::std_types::RResult::ROk(v), - Err(e) => abi_stable::std_types::RResult::RErr( - abi_stable::std_types::RString::from(e.to_string()), + Ok(v) => $crate::ffi_option::FfiResult::Ok(v), + Err(e) => $crate::ffi_option::FfiResult::Err( + stabby::alloc::string::String::from(e.to_string().as_str()), ), } }; } -/// This macro is a helpful conversion utility to convert from a DataFusion Result to an abi_stable::RResult -/// and to also call return when it is an error. Since you cannot use `?` on an RResult, this is designed +/// This macro is a helpful conversion utility to convert from a DataFusion Result to an FFIResult +/// and to also call return when it is an error. Since you cannot use `?` on an FFIResult, this is designed /// to mimic the pattern. #[macro_export] macro_rules! rresult_return { @@ -67,8 +69,8 @@ macro_rules! rresult_return { match $x { Ok(v) => v, Err(e) => { - return abi_stable::std_types::RResult::RErr( - abi_stable::std_types::RString::from(e.to_string()), + return $crate::ffi_option::FfiResult::Err( + stabby::alloc::string::String::from(e.to_string().as_str()), ) } } @@ -79,7 +81,7 @@ macro_rules! rresult_return { /// FFI friendly counterpart, [`WrappedSchema`] pub fn vec_fieldref_to_rvec_wrapped( fields: &[FieldRef], -) -> Result, arrow::error::ArrowError> { +) -> Result, arrow::error::ArrowError> { Ok(fields .iter() .map(FFI_ArrowSchema::try_from) @@ -92,7 +94,7 @@ pub fn vec_fieldref_to_rvec_wrapped( /// This is a utility function to convert an FFI friendly vector of [`WrappedSchema`] /// to their equivalent [`Field`]. pub fn rvec_wrapped_to_vec_fieldref( - fields: &RVec, + fields: &SVec, ) -> Result, arrow::error::ArrowError> { fields .iter() @@ -104,7 +106,7 @@ pub fn rvec_wrapped_to_vec_fieldref( /// FFI friendly counterpart, [`WrappedSchema`] pub fn vec_datatype_to_rvec_wrapped( data_types: &[DataType], -) -> Result, arrow::error::ArrowError> { +) -> Result, arrow::error::ArrowError> { Ok(data_types .iter() .map(FFI_ArrowSchema::try_from) @@ -117,7 +119,7 @@ pub fn vec_datatype_to_rvec_wrapped( /// This is a utility function to convert an FFI friendly vector of [`WrappedSchema`] /// to their equivalent [`DataType`]. pub fn rvec_wrapped_to_vec_datatype( - data_types: &RVec, + data_types: &SVec, ) -> Result, arrow::error::ArrowError> { data_types .iter() @@ -129,12 +131,13 @@ pub fn rvec_wrapped_to_vec_datatype( pub(crate) mod tests { use std::sync::Arc; - use abi_stable::std_types::{RResult, RString}; use datafusion::error::DataFusionError; use datafusion::prelude::SessionContext; use datafusion_execution::TaskContextProvider; + use stabby::alloc::string::String as SString; use crate::execution::FFI_TaskContextProvider; + use crate::ffi_option::FfiResult; use crate::util::FFIResult; pub(crate) fn test_session_and_ctx() -> (Arc, FFI_TaskContextProvider) @@ -147,7 +150,7 @@ pub(crate) mod tests { } fn wrap_result(result: Result) -> FFIResult { - RResult::ROk(rresult_return!(result)) + FfiResult::Ok(rresult_return!(result)) } #[test] @@ -155,14 +158,12 @@ pub(crate) mod tests { const VALID_VALUE: &str = "valid_value"; const ERROR_VALUE: &str = "error_value"; - let ok_r_result: FFIResult = - RResult::ROk(VALID_VALUE.to_string().into()); - let err_r_result: FFIResult = - RResult::RErr(ERROR_VALUE.to_string().into()); + let ok_r_result: FFIResult = FfiResult::Ok(SString::from(VALID_VALUE)); + let err_r_result: FFIResult = FfiResult::Err(SString::from(ERROR_VALUE)); let returned_ok_result = df_result!(ok_r_result); assert!(returned_ok_result.is_ok()); - assert!(returned_ok_result.unwrap().to_string() == VALID_VALUE); + assert!(*returned_ok_result.unwrap() == *VALID_VALUE); let returned_err_result = df_result!(err_r_result); assert!(returned_err_result.is_err()); @@ -176,13 +177,16 @@ pub(crate) mod tests { datafusion_common::ffi_err!("{ERROR_VALUE}"); let returned_ok_r_result = wrap_result(ok_result); - assert!(returned_ok_r_result == RResult::ROk(VALID_VALUE.into())); + let std_result: Result = returned_ok_r_result.into(); + assert!(std_result == Ok(VALID_VALUE.into())); let returned_err_r_result = wrap_result(err_result); - assert!(returned_err_r_result.is_err()); + let std_result: Result = returned_err_r_result.into(); + assert!(std_result.is_err()); assert!( - returned_err_r_result + std_result .unwrap_err() + .as_str() .starts_with(format!("FFI error: {ERROR_VALUE}").as_str()) ); } diff --git a/datafusion/ffi/src/volatility.rs b/datafusion/ffi/src/volatility.rs index bc714ae59587d..34934972a98d6 100644 --- a/datafusion/ffi/src/volatility.rs +++ b/datafusion/ffi/src/volatility.rs @@ -15,11 +15,12 @@ // specific language governing permissions and limitations // under the License. -use abi_stable::StableAbi; use datafusion_expr::Volatility; -#[repr(C)] -#[derive(Debug, StableAbi, Clone)] +#[stabby::stabby] +#[repr(u8)] +#[derive(Debug, Clone)] +#[expect(non_camel_case_types)] pub enum FFI_Volatility { Immutable, Stable, diff --git a/datafusion/ffi/tests/ffi_catalog.rs b/datafusion/ffi/tests/ffi_catalog.rs index 28bb5f406f53f..5a6547b8b0ebd 100644 --- a/datafusion/ffi/tests/ffi_catalog.rs +++ b/datafusion/ffi/tests/ffi_catalog.rs @@ -24,7 +24,6 @@ mod tests { use std::sync::Arc; use datafusion::catalog::{CatalogProvider, CatalogProviderList}; - use datafusion_common::DataFusionError; use datafusion_ffi::tests::utils::get_module; #[tokio::test] @@ -32,13 +31,7 @@ mod tests { let module = get_module()?; let (ctx, codec) = super::utils::ctx_and_codec(); - let ffi_catalog = - module - .create_catalog() - .ok_or(DataFusionError::NotImplemented( - "External catalog provider failed to implement create_catalog" - .to_string(), - ))?(codec); + let ffi_catalog = (module.create_catalog)(codec); let foreign_catalog: Arc = (&ffi_catalog).into(); let _ = ctx.register_catalog("fruit", foreign_catalog); @@ -59,13 +52,7 @@ mod tests { let module = get_module()?; let (ctx, codec) = super::utils::ctx_and_codec(); - let ffi_catalog_list = - module - .create_catalog_list() - .ok_or(DataFusionError::NotImplemented( - "External catalog provider failed to implement create_catalog_list" - .to_string(), - ))?(codec); + let ffi_catalog_list = (module.create_catalog_list)(codec); let foreign_catalog_list: Arc = (&ffi_catalog_list).into(); diff --git a/datafusion/ffi/tests/ffi_config.rs b/datafusion/ffi/tests/ffi_config.rs index ca0a3e31e8de6..4e5ad56722fe3 100644 --- a/datafusion/ffi/tests/ffi_config.rs +++ b/datafusion/ffi/tests/ffi_config.rs @@ -19,7 +19,7 @@ /// when the feature integration-tests is built #[cfg(feature = "integration-tests")] mod tests { - use datafusion::error::{DataFusionError, Result}; + use datafusion::error::Result; use datafusion_common::ScalarValue; use datafusion_common::config::{ConfigOptions, TableOptions}; use datafusion_execution::config::SessionConfig; @@ -31,13 +31,7 @@ mod tests { fn test_ffi_config_options_extension() -> Result<()> { let module = get_module()?; - let extension_options = - module - .create_extension_options() - .ok_or(DataFusionError::NotImplemented( - "External test library failed to implement create_extension_options" - .to_string(), - ))?(); + let extension_options = (module.create_extension_options)(); let mut config = ConfigOptions::new(); config.extensions.insert(extension_options); @@ -61,13 +55,7 @@ mod tests { fn test_ffi_table_options_extension() -> Result<()> { let module = get_module()?; - let extension_options = - module - .create_extension_options() - .ok_or(DataFusionError::NotImplemented( - "External test library failed to implement create_extension_options" - .to_string(), - ))?(); + let extension_options = (module.create_extension_options)(); let mut table_options = TableOptions::new(); table_options.extensions.insert(extension_options); @@ -92,13 +80,7 @@ mod tests { fn test_ffi_session_config_options_extension() -> Result<()> { let module = get_module()?; - let extension_options = - module - .create_extension_options() - .ok_or(DataFusionError::NotImplemented( - "External test library failed to implement create_extension_options" - .to_string(), - ))?(); + let extension_options = (module.create_extension_options)(); let mut config = SessionConfig::new().with_option_extension(extension_options); diff --git a/datafusion/ffi/tests/ffi_integration.rs b/datafusion/ffi/tests/ffi_integration.rs index 80538d4f92fb1..6e7fd6a20f7a0 100644 --- a/datafusion/ffi/tests/ffi_integration.rs +++ b/datafusion/ffi/tests/ffi_integration.rs @@ -26,7 +26,7 @@ mod tests { use arrow::datatypes::Schema; use datafusion::catalog::{TableProvider, TableProviderFactory}; - use datafusion::error::{DataFusionError, Result}; + use datafusion::error::Result; use datafusion_common::TableReference; use datafusion_common::ToDFSchema; use datafusion_expr::CreateExternalTable; @@ -42,11 +42,7 @@ mod tests { // By calling the code below, the table provided will be created within // the module's code. - let ffi_table_provider = table_provider_module.create_table().ok_or( - DataFusionError::NotImplemented( - "External table provider failed to implement create_table".to_string(), - ), - )?(synchronous, codec); + let ffi_table_provider = (table_provider_module.create_table)(synchronous, codec); // In order to access the table provider within this executable, we need to // turn it into a `TableProvider`. @@ -80,11 +76,8 @@ mod tests { let table_provider_module = get_module()?; let (ctx, codec) = super::utils::ctx_and_codec(); - let ffi_table_provider_factory = table_provider_module - .create_table_factory() - .ok_or(DataFusionError::NotImplemented( - "External table provider factory failed to implement create".to_string(), - ))?(codec); + let ffi_table_provider_factory = + (table_provider_module.create_table_factory)(codec); let foreign_table_provider_factory: Arc = (&ffi_table_provider_factory).into(); diff --git a/datafusion/ffi/tests/ffi_udaf.rs b/datafusion/ffi/tests/ffi_udaf.rs index f219979a85062..7df3404d7421b 100644 --- a/datafusion/ffi/tests/ffi_udaf.rs +++ b/datafusion/ffi/tests/ffi_udaf.rs @@ -23,7 +23,7 @@ mod tests { use arrow::array::Float64Array; use datafusion::common::record_batch; - use datafusion::error::{DataFusionError, Result}; + use datafusion::error::Result; use datafusion::logical_expr::{AggregateUDF, AggregateUDFImpl}; use datafusion::prelude::{SessionContext, col}; use datafusion_catalog::MemTable; @@ -34,12 +34,7 @@ mod tests { async fn test_ffi_udaf() -> Result<()> { let module = get_module()?; - let ffi_sum_func = - module - .create_sum_udaf() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_udaf".to_string(), - ))?(); + let ffi_sum_func = (module.create_sum_udaf)(); let foreign_sum_func: Arc = (&ffi_sum_func).into(); let udaf = AggregateUDF::new_from_shared_impl(foreign_sum_func); @@ -76,12 +71,7 @@ mod tests { async fn test_ffi_grouping_udaf() -> Result<()> { let module = get_module()?; - let ffi_stddev_func = - module - .create_stddev_udaf() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_udaf".to_string(), - ))?(); + let ffi_stddev_func = (module.create_stddev_udaf)(); let foreign_stddev_func: Arc = (&ffi_stddev_func).into(); let udaf = AggregateUDF::new_from_shared_impl(foreign_stddev_func); @@ -137,25 +127,14 @@ mod tests { async fn udf_as_input_to_udf() -> Result<()> { let module = get_module()?; - let ffi_abs_func = - module - .create_scalar_udf() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_scalar_udf" - .to_string(), - ))?(); + let ffi_abs_func = (module.create_scalar_udf)(); let foreign_abs_func: Arc = (&ffi_abs_func).into(); let abs_udf = ScalarUDF::new_from_shared_impl(foreign_abs_func); let ctx = SessionContext::new(); ctx.deregister_udf("abs"); - let ffi_sum_func = - module - .create_sum_udaf() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_udaf".to_string(), - ))?(); + let ffi_sum_func = (module.create_sum_udaf)(); let foreign_sum_func: Arc = (&ffi_sum_func).into(); let udaf = AggregateUDF::new_from_shared_impl(foreign_sum_func); diff --git a/datafusion/ffi/tests/ffi_udf.rs b/datafusion/ffi/tests/ffi_udf.rs index 02dfba599f316..6e6cb31f53133 100644 --- a/datafusion/ffi/tests/ffi_udf.rs +++ b/datafusion/ffi/tests/ffi_udf.rs @@ -22,7 +22,7 @@ mod tests { use arrow::array::{Array, AsArray}; use arrow::datatypes::DataType; use datafusion::common::record_batch; - use datafusion::error::{DataFusionError, Result}; + use datafusion::error::Result; use datafusion::logical_expr::{ScalarUDF, ScalarUDFImpl}; use datafusion::prelude::{SessionContext, col}; use datafusion_execution::config::SessionConfig; @@ -38,13 +38,7 @@ mod tests { async fn test_scalar_udf() -> Result<()> { let module = get_module()?; - let ffi_abs_func = - module - .create_scalar_udf() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_scalar_udf" - .to_string(), - ))?(); + let ffi_abs_func = (module.create_scalar_udf)(); let foreign_abs_func: Arc = (&ffi_abs_func).into(); let udf = ScalarUDF::new_from_shared_impl(foreign_abs_func); @@ -76,13 +70,7 @@ mod tests { async fn test_nullary_scalar_udf() -> Result<()> { let module = get_module()?; - let ffi_abs_func = - module - .create_nullary_udf() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_scalar_udf" - .to_string(), - ))?(); + let ffi_abs_func = (module.create_nullary_udf)(); let foreign_abs_func: Arc = (&ffi_abs_func).into(); let udf = ScalarUDF::new_from_shared_impl(foreign_abs_func); @@ -107,12 +95,7 @@ mod tests { async fn test_config_on_scalar_udf() -> Result<()> { let module = get_module()?; - let ffi_udf = - module - .create_timezone_udf() - .ok_or(DataFusionError::NotImplemented( - "External module failed to implement create_timezone_udf".to_string(), - ))?(); + let ffi_udf = (module.create_timezone_udf)(); let foreign_udf: Arc = (&ffi_udf).into(); let udf = ScalarUDF::new_from_shared_impl(foreign_udf); diff --git a/datafusion/ffi/tests/ffi_udtf.rs b/datafusion/ffi/tests/ffi_udtf.rs index ab7818932959c..69e5de90e1364 100644 --- a/datafusion/ffi/tests/ffi_udtf.rs +++ b/datafusion/ffi/tests/ffi_udtf.rs @@ -26,7 +26,7 @@ mod tests { use arrow::array::{ArrayRef, create_array}; use datafusion::catalog::TableFunctionImpl; - use datafusion::error::{DataFusionError, Result}; + use datafusion::error::Result; use datafusion_ffi::tests::utils::get_module; /// This test validates that we can load an external module and use a scalar @@ -37,12 +37,7 @@ mod tests { let module = get_module()?; let (ctx, codec) = super::utils::ctx_and_codec(); - let ffi_table_func = module - .create_table_function() - .ok_or(DataFusionError::NotImplemented( - "External table function provider failed to implement create_table_function" - .to_string(), - ))?(codec); + let ffi_table_func = (module.create_table_function)(codec); let foreign_table_func: Arc = ffi_table_func.into(); ctx.register_udtf("my_range", foreign_table_func); diff --git a/datafusion/ffi/tests/ffi_udwf.rs b/datafusion/ffi/tests/ffi_udwf.rs index c4e889b796008..66f2621d5fe63 100644 --- a/datafusion/ffi/tests/ffi_udwf.rs +++ b/datafusion/ffi/tests/ffi_udwf.rs @@ -22,7 +22,7 @@ mod tests { use std::sync::Arc; use arrow::array::{ArrayRef, create_array}; - use datafusion::error::{DataFusionError, Result}; + use datafusion::error::Result; use datafusion::logical_expr::expr::Sort; use datafusion::logical_expr::{ExprFunctionExt, WindowUDF, WindowUDFImpl, col}; use datafusion::prelude::SessionContext; @@ -33,13 +33,7 @@ mod tests { async fn test_rank_udwf() -> Result<()> { let module = get_module()?; - let ffi_rank_func = - module - .create_rank_udwf() - .ok_or(DataFusionError::NotImplemented( - "External table provider failed to implement create_scalar_udf" - .to_string(), - ))?(); + let ffi_rank_func = (module.create_rank_udwf)(); let foreign_rank_func: Arc = (&ffi_rank_func).into(); let udwf = WindowUDF::new_from_shared_impl(foreign_rank_func);