Preserve LogicalPlan to avoid clone for analyzer
- 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?
Step forward to #4628
I found that we need to clone plan if analyzer has context error
https://github.com/apache/datafusion/blob/08c5345e932f1c5c948751e0d06b1fd99e174efa/datafusion/core/src/execution/session_state.rs#L689
https://github.com/apache/datafusion/blob/08c5345e932f1c5c948751e0d06b1fd99e174efa/datafusion/core/src/execution/session_state.rs#L706
https://github.com/apache/datafusion/blob/08c5345e932f1c5c948751e0d06b1fd99e174efa/datafusion/core/src/execution/session_state.rs#L735
To avoid cloning, we need to preserve original logical plan in analzyer.
### Describe the solution you'd like
1. Introduce AnalyzerResult for analyzer check to preserve logical plan
```rust
pub enum AnalyzedResult {
Analyzed(LogicalPlan),
ContextError((LogicalPlan, DataFusionError)), // Original plan and context error
Err(DataFusionError)
}
```
It seems we need to modify trait for this approach
```rust
pub trait AnalyzerRule {
/// Rewrite `plan`
fn analyze(&self, plan: LogicalPlan, config: &ConfigOptions) -> Result;
// New one
fn analyze(&self, plan: LogicalPlan, config: &ConfigOptions) -> AnalyzerResult;
/// A human readable name for this analyzer rule
fn name(&self) -> &str;
}
```
2. Use `TransformedResult`
### Describe alternatives you've considered
Alternative approach is deep clone if analyzer failed, it is not a bad idea if failing is rare.
### Additional context
_No response_
Contributor guide
Research direction
Start in datafusion/core/src/execution/session_state.rs at the referenced analyzer call sites, then trace the AnalyzerRule trait and the existing TransformedResult usage. Define the analyzer result flow so the original LogicalPlan is retained when a context error occurs, and verify that the analyzer path no longer needs to clone the plan in that case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, performance
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100