Why do QueryPlanner and PhysicalPlanner exist as independent concepts?
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### 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`.
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 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_
Contributor guide
Research direction
Start with SessionStateBuilder, QueryPlanner, PhysicalPlanner, DefaultQueryPlanner, and DefaultPhysicalPlanner, then trace how ExtensionPlanner is configured for LogicalPlan::Extension. Compare the proposed trait relationships and document the design constraints; done means the relationship is clear and extension planners have an agreed configuration path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design, compilers
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100