mingmwang opened a new issue, #2175:
URL: https://github.com/apache/arrow-datafusion/issues/2175
**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<Expr>, String),
Column(Column),
............
AggregateFunction {
fun: aggregate_function::AggregateFunction,
args: Vec<Expr>,
distinct: bool,
},
WindowFunction {
fun: window_function::WindowFunction,
args: Vec<Expr>,
partition_by: Vec<Expr>,
order_by: Vec<Expr>,
window_frame: Option<window_frame::WindowFrame>,
},
............
}
````
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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
