timsaucer commented on code in PR #1677:
URL:
https://github.com/apache/datafusion-python/pull/1677#discussion_r3738310240
##########
.github/workflows/build.yml:
##########
@@ -196,6 +196,16 @@ jobs:
args: --out dist
rustup-components: rust-std
+ - name: Build FFI query planner test library
+ if: matrix.python-tag == 'abi3'
+ uses: PyO3/maturin-action@v1
+ with:
+ target: x86_64-unknown-linux-gnu
+ manylinux: "2_28"
+ working-directory: examples/datafusion-ffi-query-planner-example
+ args: --out dist
+ rustup-components: rust-std
Review Comment:
In order to prove that the 3 library approach works where we have different
codecs and different execution plans provided, we are adding a second test
library. This way we can make sure there is no accidental ability to reach into
a foreign code block.
##########
crates/core/src/context.rs:
##########
@@ -1385,6 +1453,19 @@ impl PySessionContext {
create_logical_extension_capsule(py, ffi.as_ref())
}
+ pub fn __datafusion_query_planner__<'py>(
+ &self,
+ py: Python<'py>,
+ ) -> PyResult<Bound<'py, PyCapsule>> {
Review Comment:
We need our session context to export it's own query planner because we have
a use case where one query planner can depend on another. This is already
supported by datafusion-distributed, so we want to be certain we support it
here.
##########
crates/core/src/context.rs:
##########
@@ -221,6 +227,42 @@ impl PySessionConfig {
}
}
+/// Adapts an FFI planner to the Tokio runtime owned by datafusion-python.
+///
+/// Upstream's `ForeignQueryPlanner` cannot recover the runtime handle from the
+/// `QueryPlanner` trait, so embedders that own the runtime must call
+/// `create_physical_plan_with_session_runtime` directly.
+#[derive(Debug, Clone)]
+struct RuntimeAwareQueryPlanner {
+ planner: FFI_QueryPlanner,
+}
Review Comment:
As the docstring says, the purpose of this is to make sure we attach the
runtime handle when needed.
##########
crates/core/src/context.rs:
##########
@@ -1211,6 +1253,32 @@ impl PySessionContext {
Ok(())
}
+ pub fn with_query_planner(&self, planner: Bound<'_, PyAny>) ->
PyDataFusionResult<Self> {
Review Comment:
This API is the main reason for this PR. Here we allow changing out the
default query planner with a user provided query planner.
##########
examples/datafusion-ffi-query-planner-example/src/config.rs:
##########
@@ -0,0 +1,112 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::any::Any;
+
+use datafusion_common::config::{
+ ConfigEntry, ConfigExtension, ConfigField, ExtensionOptions, Visit,
+};
+use datafusion_common::{DataFusionError, config_err};
+use datafusion_ffi::config::extension_options::FFI_ExtensionOptions;
+use pyo3::exceptions::PyRuntimeError;
+use pyo3::prelude::*;
+use pyo3::types::PyCapsule;
+
+#[pyclass(
+ from_py_object,
+ name = "PlannerConfig",
+ module = "datafusion_ffi_query_planner_example",
+ subclass
+)]
+#[derive(Clone, Debug)]
+pub(crate) struct PlannerConfig {
+ pub max_rows: usize,
+}
Review Comment:
I'm adding this to the query planner example because it's a very common
pattern that we will need custom configs for the query planner, so it is
reasonable to need insurance that configs pass over the FFI boundary properly
and to use as a demonstration to anyone who is providing such a library.
--
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]
