[Discuss] Different implementation style between Expr, LogicalPlan and ExecutionPlan
- 主要言語
- Rust
- スター
- 9.3k
- フォーク
- 2.4k
- 平均マージ
- 3日 11時間
- マージ済み PR(30日)
- 360
説明
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
In current DataFusion code base, `Expr`, `LogicalPlan` and `ExecutionPlan` are represented in different ways. Actually they are all tree based structures. `Expr` and `LogicalPlan` are Enums, `ExecutionPlan` is a Trait.
The LogicalPlan enum wrapped the different logical operator structs like Projection and Filter, etc.
````
pub enum LogicalPlan {
Projection(Projection),
Filter(Filter),
............
}
````
But the Expr enum doesn't wrap the expression structs and define the different expressions directly in the enum.
````
pub enum Expr {
Alias(Box, String),
Column(Column),
............
AggregateFunction {
fun: aggregate_function::AggregateFunction,
args: Vec,
distinct: bool,
},
WindowFunction {
fun: window_function::WindowFunction,
args: Vec,
partition_by: Vec,
order_by: Vec,
window_frame: Option,
},
............
}
````
I think we should unify the coding style, at least the Expr and LogicalPlan representations should follow the same style. And for physical ExecutionPlan, it is Trait/Trait Objects, I would prefer to use Enum also. Want to hear thoughts from other members.
**Describe the solution you'd like**
A clear and concise description of what you want to happen.
**Describe alternatives you've considered**
A clear and concise description of any alternative solutions or features you've considered.
**Additional context**
Add any other context or screenshots about the feature request here.
コントリビューションガイド
調査の方向性
まず、issueで説明されているExpr、LogicalPlan、ExecutionPlanの定義を比較し、周辺のツリー表現を確認します。このissueは、範囲が限定された実装タスクではなく設計に関する議論です。完了には、統一された表現についての合意と、定義された移行範囲が必要です。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- rust
- 領域
- data-engineering
- issue の種類
- リファクタリング
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 活発さ
- 停滞
- 明瞭さ
- 説明が足りない
- 初心者へのやさしさ
- 20/100