timsaucer commented on code in PR #19281:
URL: https://github.com/apache/datafusion/pull/19281#discussion_r2613622019


##########
datafusion/ffi/src/table_provider.rs:
##########
@@ -221,44 +215,49 @@ fn supports_filters_pushdown_internal(
 unsafe extern "C" fn supports_filters_pushdown_fn_wrapper(
     provider: &FFI_TableProvider,
     filters_serialized: RVec<u8>,
-) -> FFIResult<RVec<FFI_TableProviderFilterPushDown>> {
-    supports_filters_pushdown_internal(provider.inner(), &filters_serialized)
-        .map_err(|e| e.to_string().into())
-        .into()
+) -> RResult<RVec<FFI_TableProviderFilterPushDown>, RString> {
+    let logical_codec: Arc<dyn LogicalExtensionCodec> = 
(&provider.logical_codec).into();
+    let task_ctx = rresult_return!(<Arc<TaskContext>>::try_from(
+        &provider.logical_codec.task_ctx_provider
+    ));
+    supports_filters_pushdown_internal(
+        provider.inner(),
+        &filters_serialized,
+        &task_ctx,
+        logical_codec.as_ref(),
+    )
+    .map_err(|e| e.to_string().into())
+    .into()
 }
 
 unsafe extern "C" fn scan_fn_wrapper(
     provider: &FFI_TableProvider,
-    session_config: &FFI_SessionConfig,
+    session: FFI_SessionRef,
     projections: RVec<usize>,
     filters_serialized: RVec<u8>,
     limit: ROption<usize>,
 ) -> FfiFuture<FFIResult<FFI_ExecutionPlan>> {
+    let task_ctx: Result<Arc<TaskContext>, DataFusionError> =
+        (&provider.logical_codec.task_ctx_provider).try_into();
     let runtime = provider.runtime().clone();
+    let logical_codec: Arc<dyn LogicalExtensionCodec> = 
(&provider.logical_codec).into();
     let internal_provider = Arc::clone(provider.inner());
-    let session_config = session_config.clone();
 
     async move {
-        let config = rresult_return!(SessionConfig::try_from(&session_config));
-        let session = SessionStateBuilder::new()
-            .with_default_features()
-            .with_config(config)
-            .build();
-        let ctx = SessionContext::new_with_state(session);
+        let foreign_session = 
rresult_return!(ForeignSession::try_from(&session));

Review Comment:
   Same as other comment



##########
datafusion/ffi/src/table_provider.rs:
##########
@@ -221,44 +215,49 @@ fn supports_filters_pushdown_internal(
 unsafe extern "C" fn supports_filters_pushdown_fn_wrapper(
     provider: &FFI_TableProvider,
     filters_serialized: RVec<u8>,
-) -> FFIResult<RVec<FFI_TableProviderFilterPushDown>> {
-    supports_filters_pushdown_internal(provider.inner(), &filters_serialized)
-        .map_err(|e| e.to_string().into())
-        .into()
+) -> RResult<RVec<FFI_TableProviderFilterPushDown>, RString> {

Review Comment:
   Switch to FFI_Result



##########
datafusion/ffi/src/table_provider.rs:
##########
@@ -267,53 +266,40 @@ unsafe extern "C" fn scan_fn_wrapper(
 
         let plan = rresult_return!(
             internal_provider
-                .scan(&ctx.state(), Some(&projections), &filters, limit.into())
+                .scan(session, Some(&projections), &filters, limit.into())
                 .await
         );
 
-        RResult::ROk(FFI_ExecutionPlan::new(
-            plan,
-            ctx.task_ctx(),
-            runtime.clone(),
-        ))
+        RResult::ROk(FFI_ExecutionPlan::new(plan, runtime.clone()))
     }
     .into_ffi()
 }
 
 unsafe extern "C" fn insert_into_fn_wrapper(
     provider: &FFI_TableProvider,
-    session_config: &FFI_SessionConfig,
+    session: FFI_SessionRef,
     input: &FFI_ExecutionPlan,
     insert_op: FFI_InsertOp,
 ) -> FfiFuture<FFIResult<FFI_ExecutionPlan>> {
     let runtime = provider.runtime().clone();
     let internal_provider = Arc::clone(provider.inner());
-    let session_config = session_config.clone();
     let input = input.clone();
 
     async move {
-        let config = rresult_return!(SessionConfig::try_from(&session_config));
-        let session = SessionStateBuilder::new()
-            .with_default_features()
-            .with_config(config)
-            .build();
-        let ctx = SessionContext::new_with_state(session);
+        let foreign_session = 
rresult_return!(ForeignSession::try_from(&session));

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



-- 
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]

Reply via email to