Skip to content

Commit 7a37ceb

Browse files
authored
feat: add append functions to ffi (delta-io#962)
This PR is a retake of delta-io#550 ## What changes are proposed in this pull request? This PR implements the ffi functions required to do appends. The main difference over delta-io#550 is how it's tested: instead of using a C based example we now opt for a rust-based test that is able to re-used various testing-related utility functions that are used in the kernel tests. To do this cleanly, i've moved several functions over to the test-utils crate.
 ## This PR affects the following public APIs - Adds new ffi functions related to appends / transactions ## How was this change tested? Using the `test_basic_append` in `ffi/src/transaction/mod.rs`. ## Main points to comment on for reviewers of Draft PR: I would like to draw the reviewer's attention to: - the concept of testing new ffi functionality through rust tests instead of a C example program - usage of `tempfile::tempdir` to create a tmp directory for test data during the test - some outstanding `TODO`s in the `test_basic_append` test Once the current approach is approved by reviewers, I can follow up with more testing: - the write schema - appending to a partitioned table
1 parent d5251c1 commit 7a37ceb

8 files changed

Lines changed: 583 additions & 65 deletions

File tree

ffi/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ serde_json = "1.0.140"
3838
test_utils = { path = "../test-utils" }
3939
tokio = { version = "1.44" }
4040
trybuild = "1.0"
41+
tempfile = "3.20.0"
42+
itertools = "0.14.0"
4143

4244
[features]
4345
default = ["default-engine"]

ffi/src/engine_data.rs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
//! EngineData related ffi code
22
3+
#[cfg(feature = "default-engine-base")]
4+
use delta_kernel::arrow;
35
#[cfg(feature = "default-engine-base")]
46
use delta_kernel::arrow::array::{
57
ffi::{FFI_ArrowArray, FFI_ArrowSchema},
6-
ArrayData, StructArray,
8+
ArrayData, RecordBatch, StructArray,
79
};
810
#[cfg(feature = "default-engine-base")]
11+
use delta_kernel::engine::arrow_data::ArrowEngineData;
12+
#[cfg(feature = "default-engine-base")]
913
use delta_kernel::DeltaResult;
1014
use delta_kernel::EngineData;
1115
use std::ffi::c_void;
@@ -82,7 +86,7 @@ pub unsafe extern "C" fn get_raw_arrow_data(
8286
fn get_raw_arrow_data_impl(data: Box<dyn EngineData>) -> DeltaResult<*mut ArrowFFIData> {
8387
let record_batch: delta_kernel::arrow::array::RecordBatch = data
8488
.into_any()
85-
.downcast::<delta_kernel::engine::arrow_data::ArrowEngineData>()
89+
.downcast::<ArrowEngineData>()
8690
.map_err(|_| delta_kernel::Error::EngineDataType("ArrowEngineData".to_string()))?
8791
.into();
8892
let sa: StructArray = record_batch.into();
@@ -93,3 +97,35 @@ fn get_raw_arrow_data_impl(data: Box<dyn EngineData>) -> DeltaResult<*mut ArrowF
9397
let ret_data = Box::new(ArrowFFIData { array, schema });
9498
Ok(Box::leak(ret_data))
9599
}
100+
101+
/// Creates engine data from Arrow C Data Interface array and schema.
102+
///
103+
/// Converts the provided Arrow C Data Interface array and schema into delta-kernel's internal
104+
/// engine data format. Note that ownership of the array is transferred to the kernel, whereas the
105+
/// ownership of the schema stays the engine's.
106+
///
107+
/// # Safety
108+
/// - `array` must be a valid FFI_ArrowArray
109+
/// - `schema` must be a valid pointer to a FFI_ArrowSchema
110+
/// - `engine` must be a valid Handle to a SharedExternEngine
111+
#[cfg(feature = "default-engine-base")]
112+
#[no_mangle]
113+
pub unsafe extern "C" fn get_engine_data(
114+
array: FFI_ArrowArray,
115+
schema: &FFI_ArrowSchema,
116+
engine: Handle<SharedExternEngine>,
117+
) -> ExternResult<Handle<ExclusiveEngineData>> {
118+
get_engine_data_impl(array, schema).into_extern_result(&engine.as_ref())
119+
}
120+
121+
#[cfg(feature = "default-engine-base")]
122+
unsafe fn get_engine_data_impl(
123+
array: FFI_ArrowArray,
124+
schema: &FFI_ArrowSchema,
125+
) -> DeltaResult<Handle<ExclusiveEngineData>> {
126+
let array_data = unsafe { arrow::array::ffi::from_ffi(array, schema) };
127+
let record_batch: RecordBatch = StructArray::from(array_data?).into();
128+
let arrow_engine_data: ArrowEngineData = record_batch.into();
129+
let engine_data: Box<dyn EngineData> = Box::new(arrow_engine_data);
130+
Ok(engine_data.into())
131+
}

ffi/src/engine_funcs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ mod tests {
223223

224224
#[test]
225225
fn test_new_expression_evaluator() {
226-
let engine = get_default_engine();
226+
let engine = get_default_engine("memory:///doesntmatter/foo");
227227
let in_schema = Arc::new(StructType::new(vec![StructField::new(
228228
"a",
229229
DataType::LONG,

ffi/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ pub mod schema;
4848
mod ffi_test_utils;
4949
#[cfg(feature = "test-ffi")]
5050
pub mod test_ffi;
51+
pub mod transaction;
5152

5253
pub(crate) type NullableCvoid = Option<NonNull<c_void>>;
5354

@@ -810,16 +811,15 @@ mod tests {
810811
}
811812
}
812813

813-
pub(crate) fn get_default_engine() -> Handle<SharedExternEngine> {
814-
let path = "memory:///doesntmatter/foo";
814+
pub(crate) fn get_default_engine(path: &str) -> Handle<SharedExternEngine> {
815815
let path = kernel_string_slice!(path);
816816
let builder = unsafe { ok_or_panic(get_engine_builder(path, allocate_err)) };
817817
unsafe { ok_or_panic(builder_build(builder)) }
818818
}
819819

820820
#[test]
821821
fn engine_builder() {
822-
let engine = get_default_engine();
822+
let engine = get_default_engine("memory:///doesntmatter/foo");
823823
unsafe {
824824
free_engine(engine);
825825
}

0 commit comments

Comments
 (0)