njsmith opened a new issue, #13943: URL: https://github.com/apache/datafusion/issues/13943
### Is your feature request related to a problem or challenge? I wrote a `LogicalPlan::Extension` and an `ExtensionPlanner` to convert it into a physical plan. Then I tried to figure out how to configure a `SessionContext` to use my extension planner. It took a little bit: Step 1: `let planner = DefaultPhysicalPlanner::with_extension_planners(vec![Arc::new(GatherPlanner)])` to get an object that impls `PhysicalPlanner` that understands my extension. Step 2: squint at `SessionStateBuilder` and surrounding environs looking for how to give it a custom `Arc<dyn PhysicalPlanner>`. Step 3: eventually realize that `SessionStateBuilder` actually wants you to give it a `QueryPlanner` instead. Step 4: `QueryPlanner` appears to be a strict subset of `PhysicalPlanner`? So I guess there should be some blanket impl or something that lets me pass in my `PhysicalPlanner` as a `QueryPlanner`? Step 5: No apparently there is no blanket impl, and `DefaultQueryPlanner` is a private struct, and it's just a trivial wrapper around `DefaultPhysicalPlanner` but without any configurability. Conclusion (?): Everyone who wants to extend planning is expected to copy/paste this code to make their own `dyn QueryPlanner` that wraps `DefaultPhysicalPlanner` but with their own configuration. Is that... right? I keep feeling like I must be missing something. ### Describe the solution you'd like I can think of a few options that would simplify: - Merge `QueryPlanner` and `PhysicalPlanner` traits into a single trait - Make `PhysicalPlanner` a sub-trait of `QueryPlanner` - Provide a blanket `impl<T: PhysicalPlanner> QueryPlanner for T` - `impl QueryPlanner for DefaultPhysicalPlanner` This is ordered with things that would have simplified my life as a new user the most on the top, and the least disruptive to the existing codebase on the bottom. I'm not sure what constraints led to the current design so I can't really judge which tradeoff is best. ### Describe alternatives you've considered _No response_ ### Additional context _No response_ -- 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: github-unsubscr...@datafusion.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
