alamb commented on a change in pull request #8097:
URL: https://github.com/apache/arrow/pull/8097#discussion_r482887593
##########
File path: rust/datafusion/src/physical_plan/planner.rs
##########
@@ -331,6 +355,16 @@ impl DefaultPhysicalPlanner {
let schema_ref = Arc::new(schema.as_ref().clone());
Ok(Arc::new(ExplainExec::new(schema_ref, stringified_plans)))
}
+ LogicalPlan::Extension { node } => {
+ let inputs = node
+ .inputs()
+ .into_iter()
+ .map(|input_plan| self.create_physical_plan(input_plan,
ctx_state))
+ .collect::<Result<Vec<_>>>()?;
+
+ self.extension_planner
Review comment:
> this implies that a single ExtensionPlanner needs to be able to handle
all custom nodes.
This is correct. I had not thought about having two different sets of
extensions registered at the same time. Do you have any specific examples of
situations where users might do that?
As you point out, given the design in this PR, users of DataFusion would
have to do some extra work to combine extensions -- namely they would
effectively have to write their own extension that invoked the others.
Given there is workaround for multiple extensions (even though it is not
very user friendly), my personal preference is to begin with supporting a
single extension for now.
I think your suggestion on how to implement multiple extension planners is
an excellent one and I will use it if we decide to support multiple set of
extensions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
