feat: update FFI TableProvider and ExecutionPlan to use FFI Session and TaskContext #19281

timsaucer · 2025-12-11T09:55:58Z

Which issue does this PR close?

Addresses part of #18671 but does not close it.

Rationale for this change

This is the major change to address the requirements of #18671. This PR combines all of the previous PRs in the issue and uses them in FFI_TableProvider and FFI_ExecutionPlan. With this change the only remaining thing to close the issue is to remove the core crate. That is a large PR that mostly just changes import paths and will be a follow up.

What changes are included in this PR?

Update all structs in the FFI crate to use the FFI_PhysicalExpr, FFI_Session, FFI_TaskContext, and FFI_LogicalExtensionCodec.
Remove creation of SessionContext within the FFI crate
Updates unit tests

Are these changes tested?

Unit tests are added. Coverage report:

Are there any user-facing changes?

Yes.

There is one major change to using the FFI crate that downstream users will need to implement. Now when creating a table provider, catalog provider, etc you need to provide a TaskContextProvider and an optional LogicalExtensionCodec. The upgrade guide has been updated.

…keep it in the privated data

… serialization

timsaucer · 2025-12-11T10:05:20Z

@paleolimbot @comphead @milenkovicm @renato2099 Almost done with the FFI epic! There is only one PR after these and it's just an import cleanup job. I'm leaving this in draft until I write up the upgrade guide, but the code here is I believe in decent shape. This pulls together all of the previous work.

timsaucer · 2025-12-12T09:12:43Z

datafusion/ffi/src/table_provider.rs

-    let default_ctx = SessionContext::new();
-    let codec = DefaultLogicalExtensionCodec {};
-


Here is the main thrust of this work. In this place and others we no longer rely on creating a default SessionContext and codec.

timsaucer · 2025-12-12T09:46:36Z

datafusion/ffi/src/plan_properties.rs

-    let codec = DefaultPhysicalExtensionCodec {};
-    let partitioning_data = rresult_return!(serialize_partitioning(
-        properties.inner().output_partitioning(),
-        &codec
-    ));
-    let output_partitioning = partitioning_data.encode_to_vec();
-
-    ROk(output_partitioning.into())


Here is one of the big wins on the physical side - no longer using the protobuf serialization.

This is cool, although my reading of this means that any ExecutionPlan passed via FFI will not benefit from any physical optimization in the same way it would have if serialized/unserialized via protobuf. It seems like these are mostly for TableProviders where the plan being passed around would normally not participate in that anyway but I thought I would point that out.

thank you for noting that @paleolimbot ! that was not obvious to me

timsaucer

Address comments from in person review

timsaucer · 2025-12-12T09:50:29Z

datafusion/ffi/src/table_provider.rs

-    supports_filters_pushdown_internal(provider.inner(), &filters_serialized)
-        .map_err(|e| e.to_string().into())
-        .into()
+) -> RResult<RVec<FFI_TableProviderFilterPushDown>, RString> {


Switch to FFI_Result

Does this need updating?

timsaucer · 2025-12-12T09:55:43Z

datafusion/ffi/src/table_provider.rs

-            .with_config(config)
-            .build();
-        let ctx = SessionContext::new_with_state(session);
+        let foreign_session = rresult_return!(ForeignSession::try_from(&session));


ForeignSession is not cheap to create, so use a different work around to avoid creating if not neccessary.

You could possibly push this type of optimization into the ForeignSession by only doing expensive initialization if one of the methods is actually called using a OnceLock or something. If I'm remembering that PR correctly, the main issue there is that some of the session trait items don't return a Result so there's no opportunity for failure.

timsaucer · 2025-12-12T09:56:27Z

datafusion/ffi/src/table_provider.rs

-            .with_config(config)
-            .build();
-        let ctx = SessionContext::new_with_state(session);
+        let foreign_session = rresult_return!(ForeignSession::try_from(&session));


Same as other comment

paleolimbot

Such a satisfying amount of hack removal! This looks reasonable to me.

It seems like we are leaning on existing tests to cover the functionality here...I imagine it is tricky to test the situation where the structs you've listed in the upgrade guide are actually provided by a separate build and are actually passing user-defined functions that aren't in the default SessionContext as filters (perhaps datafusion-python is where those high level tests will live?)

paleolimbot · 2025-12-12T15:25:15Z

datafusion/ffi/src/plan_properties.rs

-    let codec = DefaultPhysicalExtensionCodec {};
-    let partitioning_data = rresult_return!(serialize_partitioning(
-        properties.inner().output_partitioning(),
-        &codec
-    ));
-    let output_partitioning = partitioning_data.encode_to_vec();
-
-    ROk(output_partitioning.into())


This is cool, although my reading of this means that any ExecutionPlan passed via FFI will not benefit from any physical optimization in the same way it would have if serialized/unserialized via protobuf. It seems like these are mostly for TableProviders where the plan being passed around would normally not participate in that anyway but I thought I would point that out.

paleolimbot · 2025-12-12T15:32:17Z

datafusion/ffi/src/table_provider.rs

-    supports_filters_pushdown_internal(provider.inner(), &filters_serialized)
-        .map_err(|e| e.to_string().into())
-        .into()
+) -> RResult<RVec<FFI_TableProviderFilterPushDown>, RString> {


Does this need updating?

paleolimbot · 2025-12-12T15:37:56Z

datafusion/ffi/src/table_provider.rs

-            .with_config(config)
-            .build();
-        let ctx = SessionContext::new_with_state(session);
+        let foreign_session = rresult_return!(ForeignSession::try_from(&session));


You could possibly push this type of optimization into the ForeignSession by only doing expensive initialization if one of the methods is actually called using a OnceLock or something. If I'm remembering that PR correctly, the main issue there is that some of the session trait items don't return a Result so there's no opportunity for failure.

renato2099 · 2025-12-14T18:03:01Z

datafusion/ffi/src/plan_properties.rs

-    let codec = DefaultPhysicalExtensionCodec {};
-    let partitioning_data = rresult_return!(serialize_partitioning(
-        properties.inner().output_partitioning(),
-        &codec
-    ));
-    let output_partitioning = partitioning_data.encode_to_vec();
-
-    ROk(output_partitioning.into())


thank you for noting that @paleolimbot ! that was not obvious to me

renato2099 · 2025-12-14T18:34:40Z

datafusion/ffi/src/plan_properties.rs

-        // TODO Extend FFI to get the registry and codex
-        let default_ctx = SessionContext::new();
-        let task_context = default_ctx.task_ctx();
-        let codex = DefaultPhysicalExtensionCodec {};


just for my own learning, before we were setting a specific codex here, do we need to worry about it? should we be using the one that is coming with from ffi ?

timsaucer added 3 commits December 11, 2025 10:21

Use the FFI_TaskContext with execution plans so we no longer have to …

8205657

…keep it in the privated data

Now that we have these physical structs, use them instead of protobuf…

1e04bcc

… serialization

Passing through logical codec because it is needed by table providers

563b446

github-actions bot added the ffi Changes to the ffi crate label Dec 11, 2025

timsaucer mentioned this pull request Dec 11, 2025

Support user defined functions as expressions in FFI calls #18671

Open

Documentation updates

6ef56f3

Update documentation about how to upgrade FFI work

ca4ef15

github-actions bot added the documentation Improvements or additions to documentation label Dec 12, 2025

timsaucer added the api change Changes the API exposed to users of the crate label Dec 12, 2025

timsaucer commented Dec 12, 2025

View reviewed changes

paleolimbot approved these changes Dec 12, 2025

View reviewed changes

renato2099 approved these changes Dec 14, 2025

View reviewed changes

timsaucer marked this pull request as ready for review December 16, 2025 19:29

		let default_ctx = SessionContext::new();
		let codec = DefaultLogicalExtensionCodec {};

feat: update FFI TableProvider and ExecutionPlan to use FFI Session and TaskContext #19281

Are you sure you want to change the base?

feat: update FFI TableProvider and ExecutionPlan to use FFI Session and TaskContext #19281

Uh oh!

Conversation

timsaucer commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

timsaucer commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timsaucer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

timsaucer commented Dec 11, 2025 •

edited

Loading