milenkovicm commented on code in PR #18688:
URL: https://github.com/apache/datafusion/pull/18688#discussion_r2532122063
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -185,6 +187,7 @@ pub struct SessionState {
/// It will be invoked on `CREATE FUNCTION` statements.
/// thus, changing dialect o PostgreSql is required
function_factory: Option<Arc<dyn FunctionFactory>>,
+ cache_producer: Option<Arc<dyn CacheProducer>>,
Review Comment:
same like previous, `cache_factory` maybe?
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -468,6 +468,12 @@ impl SessionContext {
self
}
+ /// Register a [`CacheProducer`] to provide custom caching strategy
+ pub fn with_cache_producer(self, cache_producer: Arc<dyn CacheProducer>)
-> Self {
Review Comment:
would it make sense to call it `cache_factory` rather than `cache_producer`
to align with other similar methods like `function_factory`?
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -468,6 +468,12 @@ impl SessionContext {
self
}
+ /// Register a [`CacheProducer`] to provide custom caching strategy
+ pub fn with_cache_producer(self, cache_producer: Arc<dyn CacheProducer>)
-> Self {
Review Comment:
do we need to expose this on session context, its not going to be used that
often, maybe just having it on session state would be enough ?
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -1884,6 +1890,15 @@ pub enum RegisterFunction {
Table(String, Arc<dyn TableFunctionImpl>),
}
+/// Interface for applying a custom caching strategy.
+/// Implement this trait and register via [`SessionState`]
+/// to create a custom logical node for caching.
+pub trait CacheProducer: Debug + Sync + Send {
Review Comment:
just for discussion, do we need to create new trait for this or simple
closure `|SessionState, LogicalPlan| -> LogicalPlan` would do?
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -1884,6 +1890,21 @@ pub enum RegisterFunction {
Table(String, Arc<dyn TableFunctionImpl>),
}
+/// Interface for applying a custom caching strategy.
+/// Implement this trait and register via [`SessionState`]
+/// to create a custom logical node for caching.
+/// Additionally, a custom
[`crate::physical_planner::ExtensionPlanner`]/[`QueryPlanner`]
+/// need to be implemented to handle plans containing such node.
+pub trait CacheProducer: Debug + Sync + Send {
+ /// Create a custom logical node for caching
+ /// given a logical plan (of DF to cache) and a session state.
+ fn create(
+ &self,
+ plan: LogicalPlan,
+ session_state: &SessionState,
+ ) -> Result<Arc<dyn UserDefinedLogicalNode>>;
Review Comment:
maybe it would make sense to return `LogicalNode` here, so we do not limit
just on `UserDefinedLogicalNode`?
--
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]
