ahirner commented on code in PR #13183: URL: https://github.com/apache/datafusion/pull/13183#discussion_r1823389188
########## datafusion/ffi/README.md: ########## @@ -0,0 +1,81 @@ +<!--- + 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. +--> + +# `datafusion-ffi`: Apache DataFusion Foreign Function Interface + +This crate contains code to allow interoperability of Apache [DataFusion] +with functions from other languages using a stable interface. + +See [API Docs] for details and examples. + +We expect this crate may be used by both sides of the FFI. This allows users +to create modules that can interoperate with the necessity of using the same +version of DataFusion. The driving use case has been the `datafusion-python` +repository, but many other use cases may exist. We envision at least two +use cases. + +1. `datafusion-python` which will use the FFI to provide external services such + as a `TableProvider` without needing to re-export the entire `datafusion-python` + code base. With `datafusion-ffi` these packages do not need `datafusion-python` + as a dependency at all. +2. Users may want to create a modular interface that allows runtime loading of + libraries. + +## Struct Layout + +In this crate we have a variety of structs which closely mimic the behavior of +their internal counterparts. In the following example, we will refer to the +`TableProvider`, but the same pattern exists for other structs. + +Each of the exposted structs in this crate is provided with a variant prefixed Review Comment: ```suggestion Each of the exposed structs in this crate is provided with a variant prefixed ``` I also associate variants with enums. Maybe companion instead of variant is less ambiguous? ########## datafusion/ffi/README.md: ########## @@ -0,0 +1,81 @@ +<!--- + 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. +--> + +# `datafusion-ffi`: Apache DataFusion Foreign Function Interface + +This crate contains code to allow interoperability of Apache [DataFusion] +with functions from other languages using a stable interface. + +See [API Docs] for details and examples. + +We expect this crate may be used by both sides of the FFI. This allows users +to create modules that can interoperate with the necessity of using the same Review Comment: ```suggestion to create modules that can interoperate without the necessity of using the same ``` ########## datafusion/ffi/README.md: ########## Review Comment: A small explanation on where and how FFI table scans are scheduled in terms of rt threads would be helpful. ########## datafusion-examples/examples/ffi/ffi_module_loader/src/main.rs: ########## @@ -0,0 +1,56 @@ +// 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. + +use std::sync::Arc; + +use datafusion::{ + error::{DataFusionError, Result}, + prelude::SessionContext, +}; + +use abi_stable::library::{development_utils::compute_library_path, RootModule}; +use datafusion_ffi::table_provider::ForeignTableProvider; +use ffi_module_interface::TableProviderModuleRef; + +#[tokio::main] +async fn main() -> Result<()> { + let target: &std::path::Path = "../../../../target/".as_ref(); + let library_path = compute_library_path::<TableProviderModuleRef>(target).unwrap(); + + let table_provider_module = + TableProviderModuleRef::load_from_directory(&library_path) + .unwrap_or_else(|e| panic!("{}", e)); Review Comment: Curios why not to unwrap: does the closure produce more optimal code or address some safety concerns? ########## datafusion/ffi/README.md: ########## @@ -0,0 +1,81 @@ +<!--- + 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. +--> + +# `datafusion-ffi`: Apache DataFusion Foreign Function Interface + +This crate contains code to allow interoperability of Apache [DataFusion] +with functions from other languages using a stable interface. Review Comment: Can we already define `stable` more precisely? Given frequent major releases of `datafusion`, a user may assume they can't load an FFI implementation compiled with `42` into a binary of version `43`. OTOH, are there maybe subtle limitations regarding tokio runtime or something alike? (didn't research this topic TBH) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
