feat(c/sedona-extension): Add FFI table provider, exec plan, and synchronous stream exchange#1004
feat(c/sedona-extension): Add FFI table provider, exec plan, and synchronous stream exchange#1004paleolimbot wants to merge 54 commits into
Conversation
a7ef707 to
1fdef0c
Compare
This reverts commit 56f2ed6.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Sedona C-extension FFI layer for exporting/importing TableProvider and ExecutionPlan objects across process/runtime boundaries (Python/R/ADBC), enabling cross-SedonaDB-context DataFrame round-tripping while preserving DataFusion optimization opportunities (e.g., projection pushdown).
Changes:
- Add a new
sedona-extensioncrate implementing ABI-stable C interfaces for table providers, execution plans, expressions, and Arrow stream utilities. - Replace the previous Rust/Python stream-reader utilities with a shared
StreamingRecordBatchReaderimplementation (with optional cancellation / empty-batch skipping). - Update Python/R/ADBC integrations to export/import providers via
__sedonadb_table_provider__and add Python FFI round-trip tests.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/sedona/src/reader.rs | Removes the old SedonaStreamReader implementation. |
| rust/sedona/src/lib.rs | Stops exporting the removed reader module. |
| rust/sedona/src/dataframe.rs | Adds a Sedona DataFrame extension trait + write options (currently duplicates context.rs). |
| rust/sedona-adbc/src/statement.rs | Switches ADBC streaming export to StreamingRecordBatchReader. |
| rust/sedona-adbc/Cargo.toml | Adds sedona-extension dependency. |
| r/sedonadb/src/rust/src/dataframe.rs | Switches R streaming export to StreamingRecordBatchReader. |
| r/sedonadb/src/rust/Cargo.toml | Adds sedona-extension dependency. |
| python/sedonadb/tests/test_dataframe.py | Updates comment to new __sedonadb_table_provider__ interface name. |
| python/sedonadb/tests/test_dataframe_ffi.py | Adds end-to-end Python tests validating FFI DataFrame round-trips. |
| python/sedonadb/src/runtime.rs | Removes now-unused Rust-side “wait for future” helper. |
| python/sedonadb/src/reader.rs | Replaces Python stream reader with new_py_streaming_reader using cancellation-aware StreamingRecordBatchReader. |
| python/sedonadb/src/import_from.rs | Switches provider import to new Sedona FFI table provider capsule type. |
| python/sedonadb/src/dataframe.rs | Exports __sedonadb_table_provider__ via sedona-extension’s exported provider implementation. |
| python/sedonadb/python/sedonadb/dataframe.py | Renames Python-side provider hook to __sedonadb_table_provider__ and updates docs/comments. |
| python/sedonadb/Cargo.toml | Adds sedona-extension dependency. |
| Cargo.lock | Records new crate dependencies for sedona-extension usage. |
| c/sedona-extension/src/utils.rs | Adds Arrow stream/property utilities + StreamingRecordBatchReader implementation and tests. |
| c/sedona-extension/src/table_provider.rs | Adds exported/imported TableProvider FFI wrappers and tests. |
| c/sedona-extension/src/execution_plan.rs | Adds exported/imported ExecutionPlan FFI wrappers and tests. |
| c/sedona-extension/src/expr.rs | Adds exported/imported expression FFI wrappers and tests. |
| c/sedona-extension/src/sedona_extension.h | Extends the C header with new ABI structs and callbacks. |
| c/sedona-extension/src/lib.rs | Exposes new execution_plan, expr, table_provider, and utils modules. |
| c/sedona-extension/src/extension.rs | Adds Rust FFI struct definitions for new ABI types (error/expr/plan/provider). |
| c/sedona-extension/Cargo.toml | Adds dependencies needed for new FFI/provider/plan implementations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Test cases: (producer_sql, consumer_sql) | ||
| FFI_TEST_CASES = [ | ||
| # Simple select all | ||
| pytest.param( | ||
| "SELECT * FROM water_point", | ||
| "SELECT * FROM df_producer", | ||
| id="select_all", | ||
| ), | ||
| # Projection - select specific columns | ||
| pytest.param( | ||
| 'SELECT "OBJECTID", "FEAT_CODE" FROM water_point', | ||
| 'SELECT * FROM df_producer ORDER BY "OBJECTID" LIMIT 10', | ||
| id="projection", | ||
| ), |
There was a problem hiding this comment.
This is the test that I wanted to make sure I could write before marking this ready for review: we export one of our own DataFrames as a "table", then write a query against it. We currently push projections through the boundary but not filters, but in either case the result should be correct.
| /// \brief FFI interface for a physical execution operator | ||
| /// | ||
| /// Before using this structure, the release callback MUST be checked. | ||
| /// Instances with a NULL release callback are not valid and must not be used. | ||
| struct SedonaCExecutionPlan { | ||
| /// \brief Get the schema associated with the output of this plan | ||
| void (*get_schema)(const struct SedonaCExecutionPlan* self, struct ArrowSchema* out); | ||
|
|
||
| /// \brief Get the data type of a property | ||
| int (*get_property_schema)(const struct SedonaCExecutionPlan* self, | ||
| const char* property, struct ArrowSchema* out, | ||
| struct SedonaCError* err); | ||
|
|
||
| /// \brief Extract a serializable property from this plan | ||
| /// | ||
| /// This is used to implement PlanProperties and other values that can be | ||
| /// easily retrieved and serialized. The data type associated with the out | ||
| /// array may be retrieved with the get_property_schema callback. | ||
| int (*get_property)(const struct SedonaCExecutionPlan* self, const char* property, | ||
| struct SedonaCExecutionPlanArgs* args, struct ArrowArray* out, | ||
| struct SedonaCError* err); |
There was a problem hiding this comment.
This is the C FFI over which a scan is handed across a boundary. This will be stable once we release the next version but we might need to change it to support everything we need to support before then. I designed this part to have as few callbacks as possible to ensure we can update things in a backward-compatible way without breaking the ABI.
| /// \brief Resolve a synchronous stream for one partition from this plan | ||
| int (*execute)(const struct SedonaCExecutionPlan* self, | ||
| struct SedonaCExecutionPlanArgs* args, struct ArrowArrayStream* out, | ||
| struct SedonaCError* err); | ||
|
|
||
| /// \brief Resolve an asynchronous stream for one partition from this plan | ||
| /// | ||
| /// This is not currently implemented and must be NULL. In the future, | ||
| /// out must point to a caller-supplied struct ArrowAsyncDeviceStreamHandler | ||
| /// as specified in the Arrow C Device Async Stream specification. | ||
| int (*execute_async)(const struct SedonaCExecutionPlan* self, | ||
| struct SedonaCExecutionPlanArgs* args, void* out, | ||
| struct SedonaCError* err); |
There was a problem hiding this comment.
Currently this uses blocking ArrowArrayStream/RecordBatchReaders on both sides of the boundary; however, those are implemented in such a way that they can be handed a cancellation token on both ends (e.g., so in Python when you hit Control-C, both the producer and the consumer have a mechanism to know that a cancel was requested)..
The Async version is very verbose and we should do it someday, but not today (prototyped in #914 ).
| /// \brief ABI-stable table provider interface | ||
| /// | ||
| /// This provides a minimal interface for importing a table provider | ||
| /// across an FFI boundary in a version-agnostic manner. | ||
| /// | ||
| /// Before using this structure, the release callback MUST be checked. | ||
| /// Instances with a NULL release callback are not valid and must not be used. | ||
| struct SedonaCTableProvider { | ||
| /// \brief Get the schema of this table provider | ||
| void (*get_schema)(const struct SedonaCTableProvider* self, struct ArrowSchema* out); | ||
|
|
||
| /// \brief Get the data type of a property | ||
| int (*get_property_schema)(const struct SedonaCTableProvider* self, | ||
| const char* property, struct ArrowSchema* out, | ||
| struct SedonaCError* err); |
There was a problem hiding this comment.
This is the FFI for the table itself.
| /// A RecordBatchReader that lazily polls a SendableRecordBatchStream. | ||
| /// | ||
| /// This allows exporting a DataFusion stream over FFI without collecting all | ||
| /// batches upfront. Used when exporting execution plans across FFI boundaries. | ||
| /// | ||
| /// Internally uses a dedicated worker thread to avoid blocking issues when | ||
| /// called from within a tokio runtime context. | ||
| pub struct StreamingRecordBatchReader { | ||
| schema: SchemaRef, | ||
| /// Stream and runtime, wrapped in Option so they can be moved to the worker. | ||
| stream_and_runtime: Option<(SendableRecordBatchStream, tokio::runtime::Handle)>, | ||
| /// Worker thread state, lazily initialized on first fetch. | ||
| worker: Option<StreamWorker>, | ||
| cancel_checker: Option<CancelChecker>, | ||
| cancelled: bool, | ||
| skip_empty_batches: bool, | ||
| periodic_check_interval: Option<std::time::Duration>, | ||
| } |
There was a problem hiding this comment.
This is where we take an async SendableRecordBatchStream and export it as a RecordBatchReader that blocks on every call to next().
| /// Convert an FFI ArrowArrayStream into a SendableRecordBatchStream with cancellation support. | ||
| /// | ||
| /// The cancellation checker is called before each batch is read. If it returns | ||
| /// `true`, the stream yields a cancellation error. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// The caller must ensure that the FFI stream pointer is valid and properly | ||
| /// initialized. | ||
| pub unsafe fn ffi_stream_to_sendable_with_cancel( | ||
| ffi_stream: &mut FFI_ArrowArrayStream, | ||
| cancel_checker: Option<CancelChecker>, | ||
| ) -> Result<SendableRecordBatchStream> { |
There was a problem hiding this comment.
This is the other direction: A SendableRecordBatchStream that polls a blocking RecordBatchReader from FFI. This would also benefit from the periodic cancel checker evaluation but we didn't specifically wire that up yet and so I didn't implement it (in the other direction this had already been implemented for the Python reader and there's a mechanism to implement it for the ADBC reader, which is why I included it here).
| if obj.hasattr("__datafusion_table_provider__")? { | ||
| let provider = import_ffi_table_provider(obj)?; | ||
| if obj.hasattr("__sedonadb_table_provider__")? { | ||
| let provider = import_sedona_ffi_table_provider(obj)?; |
There was a problem hiding this comment.
This is the swap that was the target endpoint of this PR
| impl PySedonaStreamReader { | ||
| pub fn new(runtime: Arc<Runtime>, stream: SendableRecordBatchStream) -> Self { | ||
| Self { runtime, stream } | ||
| } | ||
| } |
There was a problem hiding this comment.
I replaced this reader with the reader in sedona-extension
There was a problem hiding this comment.
I also replaced this reader with the reader in sedona-extension
| struct SedonaCExpr { | ||
| /// \brief Get the data type of a property | ||
| int (*get_property_schema)(const struct SedonaCExpr* self, const char* property, | ||
| struct SedonaCError* err); | ||
|
|
||
| /// \brief Extract a serializable property from this expression | ||
| /// | ||
| /// This is used to implement PlanProperties and other values that can be | ||
| /// easily retrieved and serialized. The data type associated with the out | ||
| /// array may be retrieved with the get_property_schema callback. | ||
| int (*get_property)(const struct SedonaCExpr* self, const char* property, | ||
| const char* args, struct ArrowArray* out, struct SedonaCError* err); |
There was a problem hiding this comment.
This is a follow-up. This is how filters will get passed to get_property and scan...get_property can be queried for a spatial representation of the filter (e.g., bbox for a particular column), or a completely serialized filter in various forms (substrait, datafusion proto, maybe SQL but at the cost of some precision)
|
I havent had time to review this myself yet, but here is a review from fable/ultracode. Do with it what you will knowing its just from the AI. Correctness bugs
Efficiency
Altitude
Reuse / simplification
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| struct SedonaCError* err); | ||
|
|
||
| /// \brief Reserved for future use (must be NULL). | ||
| void* reserved; |
There was a problem hiding this comment.
these are advanced tactics heretofore unknown to me
This PR adds an FFI wrapper around a TableProvider and enough infrastructure to replace an actual use case that we can test, which is importing a
DataFramefrom a separate SedonaDB context.