start implementing FFI #91

ryan-johnson-databricks · 2023-12-13T17:19:06Z

Ability for engine to get a snapshot, traverse the schema, provide skipping predicates, and obtain the file list for scan.

Consumed by a duckdb extension (TODO: figure out how publish the extension so others can actually try this out!)

Not at all complete, but posting for ideas on how to improve it.

This took a lot of work vs. the previous hackathon because the ffi is now in a separate crate. In particular:

How to make self-standing tests for the new FFI API calls?
- Right now I manually test using the duckdb extension against a table I created locally using python+pandas+delta; I guess we should somehow be using DAT tables instead?
- Expression and schema visitor stuff requires a lot of engine-side definitions. Which for any real engine is easy -- the engine already had to define those internally. But for a test... we'll end up defining stuff just for the test, I guess? A bit clunky.
The header export via build.rs doesn't actually work, because cbindgen is unable to emit definitions for types from any crate other than the one being exported.
- Every kernel class the FFI would expose needs manual forward declarations via cbindgen configs, which could easily go out of sync as kernel continues to evolve.
- build.rs invocation suppresses all output from cbindgen so there's no indication that any of this is even a problem
- Running manually from CLI shows warnings, but then the build.rs config that provides the needed forward declarations is inaccessible

wjones127

I think we need to:

Decide on a strategy for error handling
Wrap more of the Rust API in C-friendly structs. (This will be a lot of code, and it's why I wanted the FFI bindings to be a separate crate.)

wjones127 · 2023-12-16T01:54:20Z

ffi/src/lib.rs

+// TODO: We REALLY don't want to expose the real type of default client. Doing so forces every kernel
+// API call to take either a default client or a generic client, because those are incompatible types.
+use deltakernel::client::executor::tokio::TokioBackgroundExecutor;
+type KernelDefaultTableClient = deltakernel::DefaultTableClient<TokioBackgroundExecutor>;


Yeah we should just wrap this as a generic client.

wjones127 · 2023-12-16T01:59:53Z

ffi/kernel.h

+typedef struct EngineIterator {
+  void *data;
+  /**
+   * A function that should advance the iterator and return the next time from the data
+   * If the iterator is complete, it should return null. It should be safe to
+   * call `get_next()` multiple times if it is null.
+   */
+  const void *(*get_next)(void *data);
+} EngineIterator;


I guess this was introduced earlier, but it looks like engine iterator is missing a release/free method to cleanup any allocations it may have made.

wjones127 · 2023-12-16T02:37:47Z

ffi/src/lib.rs

+#[no_mangle]
+pub unsafe extern "C" fn get_default_client(
+    path: *const c_char,
+) -> *const KernelDefaultTableClient {


A few points:

Error handling: From what I've seen, usually C APIs will return an error code to indicate whether the operation was successful. That seems to be how FUSE handles errors. In the ADBC C API, we return an error code and also have a pointer to an error struct that allows filling in more information like an error message (example function signature).

Allocating types: This function shoves the table client into an Arc, and then returns it as a raw pointer, giving the caller no way to release the data when it's done with it. That seems to be an inevitable memory leak. Instead, we should probably give the C API a struct it can allocate and we can initialize:

#[repr(C)] struct TableClient { data: *mut c_void, get_file_system_client: extern "C" fn(data: *const TableClient, out: *mut FileSystemClient) -> StatusCode, ... /// Tell the table client to release the resources release: extern "C" fn(self: *mut TableClient) -> (), } #[repr(C)] struct FileSystemClient { ... }

The FFI TableClient can then easily implement the trait kernel::TableClient by calling the function pointers. For example:

impl kernel::TableClient for TableClient { fn get_file_system_client(&self) -> Result<Arc<dyn kernel::FileSystemClient>> { // Initialize an empty FFI filesystemclient let mut fs_client = FileSystemClient::empty(); let status = self.get_file_system_client(&self, &mut fs_client); match status { Status::Ok => Ok(Arc::new(fs_client)), Status::Err => Err(Error), } } }

Then for us to provide the default client, we just need to wrap KernelDefaultTableClient in the FFI wrapper:

#[no_mangle] pub unsafe extern "C" fn get_default_client( path: *const c_char, out: *mut TableClient ) -> StatusCode { let path = unsafe { unwrap_and_parse_path_as_url(path) }; let Some(url) = path else { return std::ptr::null_mut(); }; let table_client = KernelDefaultTableClient::try_new( &url, HashMap::<String, String>::new(), Arc::new(TokioBackgroundExecutor::new()), ); out.get_file_system_client = default_client_get_fs_client; out.release = default_client_release; out.data = Arc::into_raw(Arc::new(table_client)) } unsafe extern "C" fn default_client_get_fs_client( table_client: *mut TableClient, out: *mut FileSystemClient ) -> StatusCode { todo!("wrap the filesystem in the FFI FileSystemClient and copy to provided pointer") } unsafe extern "C" fn default_client_release( table_client: *mut TableClient, ) -> StatusCode { // Converting data back to Arc so ref count will be decremented when dropped // at end of this function. let _client = unsafe { Arc::from_raw(table_client.data) } table_client.data = std::ptr::null; // Make release callback null to make this struct as having been released. table_client.release = std::ptr::null; }

Yes, that's the basic idea I settled on as well... but it's easier said than done:

If the engine wants to provide a table client, they need a way to instantiate an Arc<dyn TableClient>. The vtable emulation stuff that safer-ffi came up with seems like the right "flavor" of approach... except that it seems to only handle exporting a trait to C++, and does not provide any obvious way for C++ code to implement traits.

Even if we do manually code up the vtable for TableClient as you suggest, there's still the problem that TableClient::get_expression_handler returns Arc<dyn Expression>... same problem all over again

... and Expression::get_evaluator returns Arc<dyn ExpressionEvaluator>... so again

... and ExpressionEvaluator::evaluate method returns DeltaResult<RecordBatch>, and there's also no way to express a rust enum in C/C++. For example, safer-ffi doesn't even try to support non-trivial rust enums right now: https://getditto.github.io/safer_ffi/derive-reprc/enum.html#more-complex-enums ("Are not supported yet.")

... and even if we figure out the DeltaResult part, that RecordBatch is an arrow API class that will soon enough change to Arc<dyn DataBatch> or similar.

(and that's just one path through the tree of structs and methods)

Wrt. all the various different "handlers" or "clients" we need to return, I think in the end it will be on the order of 10-20 function pointer assigners we'll have to implement. That doesn't seem to bad all together. (fuse has about 45). I think having to have a get_file_system_client, and a get_expression_handler, and all the other get_s isn't unreasonable.

For the result stuff, I think we are stuck with the "old" way of doing things in C, which is to say, null means "error" and otherwise it's a pointer to valid data.

It would be nice if we could still return a StatusCode, with the convention that we will only have filled in an out pointer if the return code is valid, but I'm not sure we can create a null pointer to a trait object to fill in, because *mut Trait is a fat pointer.

So we'll need to have a slightly different signature in the FFI. So an EngineEvaluator would maybe look like:

#[repr(C)] struct FFIEvaluator { internal: *mut c_void, evaluate: extern "C" fn(internal: *mut c_void, data: *const EngineData) -> *const EngineData, } impl kernel::ExpressionEvaluator for FFIEvaluator { fn evaluate(&self, batch: &dyn EngineData) -> DeltaResult<Box<dyn EngineData>> { let batch: *const EngineData = batch; let result = self.evaulate(self.internal, batch); if result.is_null() { Err(Error) } else { unsafe { Box::from_raw(result) } } } }

We could potentially improve the error messages by allowing engines to set errno and using https://doc.rust-lang.org/std/io/struct.Error.html#method.last_os_error.

Yes, that looks about like what I was imagining as well. The main things on my mind:

It seemed worthwhile to wait for your EngineData PR to land, because it cleans up a lot of messes that make FFI harder (and also would churn any FFI I did try to write). I see a big block of empty time on my calendar today I hope I can use to finally review it.

I think I figured out a way to define error handling at (FFI) engine interface level. Basically, the FFI engine interface defines a method for kernel to allocate an exception object, which kernel guarantees to immediately return to the engine (no other intervening code that could fail and cause the error to leak). That would allow an idiom similar to rust Result. An engine that supports C++17 (**) could even use `std::variant<T, EngineError*>. But prototyping that needs to wait until we figure out the FFI engine interface in general.

How can we define our FFI engine interface API in such a way that engine actually has the choice to request a default client from kernel or use its own? Ideally, engine even be able to mix and match (e.g. use native parquet+json readers, but leverage defaults for expression eval etc). This seems tricky, because we don't want to add so many layers of indirection that would come from "just" doing a round trip through an engine vtable format. Especially when it comes to error handling, where converting from kernel error to engine errors will necessarily be lossy (because it seems unwise to pass arbitrary rust exceptions across the FFI boundary).

In many ways, 3/ is my biggest concern, to be honest.

(**) Sadly, duckdb is only C++11 and so couldn't use std::variant -- which is kind of ironic because they define a SQL UNION type that is specifically described as

similar to C++17’s std::variant, Rust’s enum or the “sum type” present in most functional languages

nicklan · 2024-02-07T22:41:54Z

ffi/src/lib.rs

+
+fn visit_expression_binary(
+    state: &mut KernelExpressionVisitorState,
+    op: BinaryOperator,


Could we make BinaryOperator be #[repr(C)] (or something that we into() a BinaryOperator)? Then we could expose this function directly and not have to have all the visit_expression_X ones below.

This would work for the ones were all we do is visit_expression_binary(state, BinaryOperator::X, a, b). I know some do other things and so would need to either remain as is or have more logic in here.

Would cut down on code and api surface.

That was my original inclination as well, but:

If we require engine to pass a generic binary operator, we replace N method definitions with one method definition plus N enum variant definitions.

Even simple enums are unsafe across the FFI boundary, because Rust assumes they never contain "out of band" values, but C/C++ could send any arbitrary value that fits in the enum's backing integral type. We could "solve" that by validating the raw enum type before using it, but that's extra code and extra failure cases that both sides now have to worry about (plus it would require us to add error handling capabilities that don't currently exist).

Taken together, it seemed strictly worse than the function-based approach I ended up pursuing here.

Granted, we'll eventually need to worry about error handling, because engine could pass invalid child ids (not tracked by inflight_expressions), but that error path seems a lot less likely to reach prod because engine isn't interpreting those values. It just needs to not lose track of them.

ffi/src/lib.rs

nicklan · 2024-02-07T23:46:40Z

ffi/src/lib.rs

+#[no_mangle]
+pub unsafe extern "C" fn get_default_client(
+    path: *const c_char,
+) -> *const KernelDefaultTableClient {


Wrt. all the various different "handlers" or "clients" we need to return, I think in the end it will be on the order of 10-20 function pointer assigners we'll have to implement. That doesn't seem to bad all together. (fuse has about 45). I think having to have a get_file_system_client, and a get_expression_handler, and all the other get_s isn't unreasonable.

For the result stuff, I think we are stuck with the "old" way of doing things in C, which is to say, null means "error" and otherwise it's a pointer to valid data.

It would be nice if we could still return a StatusCode, with the convention that we will only have filled in an out pointer if the return code is valid, but I'm not sure we can create a null pointer to a trait object to fill in, because *mut Trait is a fat pointer.

So we'll need to have a slightly different signature in the FFI. So an EngineEvaluator would maybe look like:

#[repr(C)] struct FFIEvaluator { internal: *mut c_void, evaluate: extern "C" fn(internal: *mut c_void, data: *const EngineData) -> *const EngineData, } impl kernel::ExpressionEvaluator for FFIEvaluator { fn evaluate(&self, batch: &dyn EngineData) -> DeltaResult<Box<dyn EngineData>> { let batch: *const EngineData = batch; let result = self.evaulate(self.internal, batch); if result.is_null() { Err(Error) } else { unsafe { Box::from_raw(result) } } } }

We could potentially improve the error messages by allowing engines to set errno and using https://doc.rust-lang.org/std/io/struct.Error.html#method.last_os_error.

nicklan

nice, this has some great ergonomic tricks for the ffi.

leaving the comments I have so far, and will continue to review.

nicklan · 2024-02-26T21:08:55Z

ffi/src/lib.rs

+    ///
+    /// The slice must be a valid (non-null) pointer, and must point to the indicated number of
+    /// valid utf8 bytes.
+    unsafe fn from_slice(slice: KernelStringSlice) -> String {


I'd have a preference for this to be more "try" oriented. If it returned a DeltaResult<String> you could use from_utf8 rather than from_utf8_unchecked

I was originally using from_utf8, but there are already so many other things that can go wrong in this unsafe code it seemed almost trite to return an ExternResult, which greatly complicates call chains (**), just for this one detail?

(**) Creating an ExternResult requires access to an AllocateError.

But, there are only three call sites for that method, only two of which change engine code, so I went ahead and updated them. LMK what you think?

ffi/src/handle.rs

nicklan · 2024-02-26T21:24:36Z

ffi/src/handle.rs

+    }
+}
+
+mod uncreate {


i'd prefer "uncreatable", since "Uncreate" sounds like something that would do the action of uncreating.

I went with "unconstructable" since that's the most accurate term for something that cannot be constructed?

nicklan · 2024-02-26T21:25:50Z

ffi/src/handle.rs

+//
+// Unlike the default (unsized) implementation, which must wrap the Arc in a Box in order to obtain
+// a thin pointer, the sized implementation can directly return a pointer to the Arc's
+// underlying. This blanket implementation applies automatically to all types that implement


nit: "the Arc's underlying [object/struct/type]"

nicklan · 2024-02-26T22:20:14Z

ffi/src/lib.rs

+    Err(*mut EngineError),
+}
+
+type AllocateErrorFn =


So as I understand it, the flow in an error case is something like:

engine calls kernel function

kernel encounters error

kernel calls this allocation function, importantly with a string slice that is only guaranteed to be valid until the allocate function returns

the engine can build up whatever error struct it wants, as long as the first field is a KernelError

kernel gets that back, and then returns it back to the engine

Now the engine has its own error type that it allocated that it can do whatever it wants with

This makes sense with wanting custom error messages. I guess we just need to decide that that's valuable enough to warrant all this machinery. The alternative being just errno style errors and:

#[repr(C)] pub enum ExternResult<T> { Ok(T), Err(KernelError), }

and all the allocate machinery can go away.

Simple error numbers never seem to be quite enough, and we'd inevitably end up having to rewire things to associate more information.

However, I have definitely wondered if it might be worth forcing engine to provide just one global extern error allocator, which we can then statically reference everywhere instead of passing it as an arg.

Would we provide that allocator in some kind of init function? in general I like the idea of not needing to pass it everywhere, but don't like the footgun of needing to call init first.

I think I'd vote to keep it this way for now, and if the passing of the allocator becomes burdensome in the connector code we can look at making it global.

Honestly, if it weren't for unit tests (with "weird engine implementation warts" as a secondary concern) I'd probably just have allocate_error be an undefined extern that the engine has to provide at link time. That would make it harder for unit tests to customize the error implementation (not sure we'd even need that), and if the engine is doing weird stuff where sometimes it wants different behaviors... also makes it hard.

ffi/src/handle.rs

ffi/src/lib.rs

ryan-johnson-databricks

Responding to comments.

ffi/src/handle.rs

ryan-johnson-databricks · 2024-02-26T23:56:43Z

ffi/src/lib.rs

+    ///
+    /// The slice must be a valid (non-null) pointer, and must point to the indicated number of
+    /// valid utf8 bytes.
+    unsafe fn from_slice(slice: KernelStringSlice) -> String {


I was originally using from_utf8, but there are already so many other things that can go wrong in this unsafe code it seemed almost trite to return an ExternResult, which greatly complicates call chains (**), just for this one detail?

(**) Creating an ExternResult requires access to an AllocateError.

ryan-johnson-databricks · 2024-02-26T23:57:49Z

ffi/src/lib.rs

+    ///
+    /// The slice must be a valid (non-null) pointer, and must point to the indicated number of
+    /// valid utf8 bytes.
+    unsafe fn from_slice(slice: KernelStringSlice) -> String {


But, there are only three call sites for that method, only two of which change engine code, so I went ahead and updated them. LMK what you think?

ffi/src/lib.rs

ryan-johnson-databricks · 2024-02-27T00:14:23Z

ffi/src/lib.rs

+    Err(*mut EngineError),
+}
+
+type AllocateErrorFn =


Simple error numbers never seem to be quite enough, and we'd inevitably end up having to rewire things to associate more information.

However, I have definitely wondered if it might be worth forcing engine to provide just one global extern error allocator, which we can then statically reference everywhere instead of passing it as an arg.

ryan-johnson-databricks · 2024-02-27T00:19:06Z

ffi/src/handle.rs

+    }
+}
+
+mod uncreate {


I went with "unconstructable" since that's the most accurate term for something that cannot be constructed?

ffi/src/lib.rs

ryan-johnson-databricks · 2024-03-25T22:07:33Z

ffi/kernel.h

We shouldn't check in the header files. They should be created as part of the build process and shipped as part of the build along with the kernel .a file.

nicklan

left a few more comments. One high level suggestion as we push to get this merged:

can we split this into a few more modules. 700 odd lines is getting a bit... bulky :)

I think probably all the error stuff can go in its own module, maybe along with the string slice stuff, or that could be on its own.

Then lib.rs should ideally just have the main "meat" functions that a connector calls.

nicklan · 2024-03-26T23:34:57Z

ffi/src/lib.rs

+        match std::str::from_utf8(slice) {
+            Ok(s) => Ok(s.to_string()),
+            Err(e) => Err(Error::generic_err(e)),
+        }


We have a Utf8Error #[from] variant for our Error, so you should be able to do:

Suggested change

match std::str::from_utf8(slice) {

Ok(s) => Ok(s.to_string()),

Err(e) => Err(Error::generic_err(e)),

}

Ok(std::str::from_utf8(slice)?.to_string())

error[E0277]: `?` couldn't convert the error to `deltakernel::Error` --> ffi/src/lib.rs:108:38 | 108 | Ok(std::str::from_utf8(slice)?.to_string()) | ^ the trait `From<std::str::Utf8Error>` is not implemented for `deltakernel::Error` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From<T>`: <deltakernel::Error as From<serde_json::error::Error>> <deltakernel::Error as From<arrow_schema::error::ArrowError>> <deltakernel::Error as From<url::ParseError>> <deltakernel::Error as From<std::io::Error>> <deltakernel::Error as From<parquet::errors::ParquetError>> <deltakernel::Error as From<FromUtf8Error>> <deltakernel::Error as From<std::num::ParseIntError>> = note: required for `Result<std::string::String, deltakernel::Error>` to implement `FromResidual<Result<Infallible, std::str::Utf8Error>>`

just going to leave this as todo for now and we can revisit if we want? (also some discussion of panic instead of errors)

hrmm, that's odd, but yeah, i think just unwrap and get rid of all of this for now

nicklan · 2024-03-26T23:37:10Z

ffi/src/lib.rs

+
+#[repr(C)]
+#[derive(Debug)]
+pub enum KernelError {


this (and the From below) feel like something that should be derived via a proc macro. it's a bit tricky because we don't want it to live in the same module that we define our deltakernel::Error variant. I suspect there's something we could do though. Probably fine to have it be future work.

scovich

Flushing comments from our discussion

scovich · 2024-03-28T18:42:01Z

ffi/cffi-test.c

We should expand this with tests that exercise the real FFI methods

scovich · 2024-03-28T18:50:50Z

ffi/src/lib.rs

+pub unsafe extern "C" fn snapshot(
+    path: KernelStringSlice,
+    table_client: *const ExternEngineInterfaceHandle,
+) -> ExternResult<*const SnapshotHandle> {


Not sure how to fix this warning, given that rust only supports extern "C" (not extern "C++"):

warning: 'snapshot' has C-linkage specified, but returns incomplete type 'ExternResult<const SnapshotHandle *>' which could be incompatible with C [-Wreturn-type-c-linkage] ExternResult<const SnapshotHandle*> snapshot(KernelStringSlice path, ^

ffi/src/lib.rs

scovich · 2024-03-28T19:05:28Z

ffi/src/lib.rs

+#[repr(C)]
+pub enum ExternResult<T> {
+    Ok(T),
+    Err(*mut EngineError),


Semantics: Kernel will always immediately return the leaked engine error to the engine (if it allocated one at all), and engine is responsible to free it.

added comment

scovich · 2024-03-28T19:06:11Z

ffi/src/lib.rs

+
+// NOTE: We can't "just" impl From<DeltaResult<T>> because we require an error allocator.
+impl<T> ExternResult<T> {
+    pub fn is_ok(&self) -> bool {


Sure would be handy to have access to these methods in C++ as well, but cbindgen doesn't seem to emit the member methods of a class.

scovich · 2024-03-28T19:36:14Z

ffi/src/lib.rs

+#[repr(C)]
+pub struct EngineSchemaVisitor {
+    // opaque state pointer
+    data: *mut c_void,


Should this use a formal handle type?
Or is it "one-shot" and so whocares?

Note: Kernel really doesn't care what type it is -- engine owns and manages it.

seemed like we decided who cares?

ffi/src/lib.rs

scovich · 2024-03-28T19:49:25Z

ffi/src/lib.rs

+    state.inflight_expressions.take(exprid)
+}
+
+fn visit_expression_binary(


could go to a support module

scovich · 2024-03-28T19:52:50Z

ffi/src/lib.rs

+/// Caller is responsible to (at most once) pass a valid pointer returned by a call to
+/// [kernel_scan_files_init].
+#[no_mangle]
+pub unsafe extern "C" fn kernel_scan_files_free(files: *mut KernelScanFileIterator) {


we should probably be consistent with drop vs. free on engine side (probably the latter is more intuitive to non-rust code)

added comment for now, lmk if we want this upgraded to an issue

ffi/src/lib.rs

…fault client and FFI wrappers for it

zachschuermann

opened issues for pending items and did a tiny bit of cleanup to offload from ryan! had pretty extensive discussion, current state LGTM and we can merge then iterate :)

zachschuermann · 2024-04-03T15:52:46Z

ffi/cffi-test.c

ffi/src/lib.rs

zachschuermann · 2024-04-03T16:31:05Z

ffi/src/lib.rs

+        match std::str::from_utf8(slice) {
+            Ok(s) => Ok(s.to_string()),
+            Err(e) => Err(Error::generic_err(e)),
+        }


error[E0277]: `?` couldn't convert the error to `deltakernel::Error` --> ffi/src/lib.rs:108:38 | 108 | Ok(std::str::from_utf8(slice)?.to_string()) | ^ the trait `From<std::str::Utf8Error>` is not implemented for `deltakernel::Error` | = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait = help: the following other types implement trait `From<T>`: <deltakernel::Error as From<serde_json::error::Error>> <deltakernel::Error as From<arrow_schema::error::ArrowError>> <deltakernel::Error as From<url::ParseError>> <deltakernel::Error as From<std::io::Error>> <deltakernel::Error as From<parquet::errors::ParquetError>> <deltakernel::Error as From<FromUtf8Error>> <deltakernel::Error as From<std::num::ParseIntError>> = note: required for `Result<std::string::String, deltakernel::Error>` to implement `FromResidual<Result<Infallible, std::str::Utf8Error>>`

zachschuermann · 2024-04-03T16:31:28Z

ffi/src/lib.rs

+        match std::str::from_utf8(slice) {
+            Ok(s) => Ok(s.to_string()),
+            Err(e) => Err(Error::generic_err(e)),
+        }


just going to leave this as todo for now and we can revisit if we want? (also some discussion of panic instead of errors)

ffi/src/lib.rs

zachschuermann · 2024-04-03T16:46:22Z

ffi/src/lib.rs

+    state.inflight_expressions.take(exprid)
+}
+
+fn visit_expression_binary(


ffi/src/lib.rs

zachschuermann · 2024-04-03T16:49:11Z

ffi/src/lib.rs

+/// Caller is responsible to (at most once) pass a valid pointer returned by a call to
+/// [kernel_scan_files_init].
+#[no_mangle]
+pub unsafe extern "C" fn kernel_scan_files_free(files: *mut KernelScanFileIterator) {


added comment for now, lmk if we want this upgraded to an issue

zachschuermann · 2024-04-03T16:51:16Z

ffi/src/lib.rs

+struct ExternEngineInterfaceVtable {
+    // Actual table client instance to use
+    client: Arc<dyn EngineInterface>,
+    allocate_error: AllocateErrorFn,


added comment

zachschuermann · 2024-04-03T16:54:22Z

oh and just opened #163

nicklan

nice! lgtm to merge then iterate. thanks!

ffi/src/lib.rs

Co-authored-by: Ryan Johnson <[email protected]>

partially implement FFI

b7e6702

ryan-johnson-databricks requested a review from nicklan December 13, 2023 17:19

Merge remote-tracking branch 'oss/main' into frj-ffi-wip

616cc3a

wjones127 reviewed Dec 16, 2023

View reviewed changes

ryan-johnson-databricks added 4 commits January 19, 2024 09:45

Merge remote-tracking branch 'oss/main' into frj-ffi-wip

9672bfc

simplify AND/OR logic

7802726

Fix FFI to match latest kernel code

8be8a37

scan files is now an iterator

ff68f72

ryan-johnson-databricks requested a review from zachschuermann January 27, 2024 23:06

merge with frj-simpler-and

8de115b

ryan-johnson-databricks requested a review from hntd187 January 29, 2024 13:57

ryan-johnson-databricks added 3 commits January 29, 2024 08:03

clippy/fmt

d04a737

cargo fmt again... this time with --all option

4623c51

Merge remote-tracking branch 'oss/main' into frj-ffi-wip

d053739

nicklan reviewed Feb 7, 2024

View reviewed changes

ryan-johnson-databricks added 3 commits February 23, 2024 16:30

start supporting basic error handling and harmonize trait passing

b0b1f62

clippy and fmt

92abf11

more string and error handling

7a2179b

nicklan reviewed Feb 26, 2024

View reviewed changes

review comments

f231ce0

ryan-johnson-databricks commented Feb 27, 2024

View reviewed changes

update header files

d068865

nicklan closed this Mar 19, 2024

ryan-johnson-databricks reopened this Mar 19, 2024

ryan-johnson-databricks added 2 commits March 25, 2024 11:06

Merge remote-tracking branch 'oss/main' into frj-ffi-wip

a3af30d

fix logical merge conflicts

0b32b3b

ryan-johnson-databricks commented Mar 25, 2024

View reviewed changes

ffi/src/lib.rs Outdated Show resolved Hide resolved

ryan-johnson-databricks commented Mar 25, 2024

View reviewed changes

remove generated header files

12304ef

scovich mentioned this pull request Mar 26, 2024

[WIP] Implement extern EngineData #152

Open

nicklan reviewed Mar 26, 2024

View reviewed changes

scovich reviewed Mar 28, 2024

View reviewed changes

zachschuermann self-assigned this Mar 29, 2024

zachschuermann changed the title ~~[WIP] Start implementing FFI~~ start implementing FFI Mar 29, 2024

zachschuermann added 2 commits April 3, 2024 09:12

new feature-flag in FFI crate 'default-client' that enables kernel de…

fbe16b1

…fault client and FFI wrappers for it

cleanup and comments

a02a94e

zachschuermann approved these changes Apr 3, 2024

View reviewed changes

fmt

6412bbb

zachschuermann requested review from nicklan, wjones127, rtyler, roeap and samansmink April 3, 2024 16:54

zachschuermann added 8 commits April 3, 2024 14:20

fix DEFINES for cbindgen feature flag

ff07d9a

add gitignore headers

447fdb4

try_from_slice just unwraps

3ce3b15

remove accidental kernel.hpp commit

a7b50dd

remove comment

245fdf3

Merge remote-tracking branch 'upstream/main' into frj-ffi-wip

887412f

fix Backtraced error

aa2a564

fmt

f947f6a

nicklan approved these changes Apr 3, 2024

View reviewed changes

zachschuermann added 3 commits April 3, 2024 17:59

fix cffi-test

f27c0b5

fix

7630cb2

try github actions?

57dfd28

scovich reviewed Apr 4, 2024

View reviewed changes

ffi/src/lib.rs Outdated Show resolved Hide resolved

update comment

78e4082

Co-authored-by: Ryan Johnson <[email protected]>

zachschuermann merged commit ee3f135 into delta-incubator:main Apr 4, 2024
8 checks passed

start implementing FFI #91

start implementing FFI #91

Conversation

ryan-johnson-databricks commented Dec 13, 2023 • edited Loading

wjones127 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryan-johnson-databricks Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicklan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryan-johnson-databricks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicklan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scovich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zachschuermann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zachschuermann commented Apr 3, 2024

nicklan left a comment

Choose a reason for hiding this comment

ryan-johnson-databricks commented Dec 13, 2023 •

edited

Loading

ryan-johnson-databricks Jan 29, 2024 •

edited

Loading