apache / apache/datafusion

Preserve LogicalPlan to avoid clone for analyzer

Open
#11,316 1 comment 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.